FR: Revising the Threading Model and Async APIs about firebase-admin-java HOT 32 CLOSED

We are making some headway with this issue. We are thinking about a 2-phase roll out plan:

Phase 1 (No breaking changes)

• Introduce new async methods that return ApiFuture instances (e.g. ApiFuture<String> createCustomToken().
• Deprecate the Task API

Phase 2 (breaking changes)

• Remove the methods that return Task instances.
• Replace them with a set of blocking methods (e.g. String createCustomToken()).

from firebase-admin-java.

@samodadela , ApiFuture extends java.util.concurrent.Future.

This article should be enough to learn how to use it.

If you still need extra help, try another articles from here.

from firebase-admin-java.

Well.. in the end I came up with this:

    SettableApiFuture<String> future = SettableApiFuture.create();
    final Query mDatabase = getDatabaseReference( firebaseProjectDatabase ).child( valueName );

    mDatabase.addListenerForSingleValueEvent( new ValueEventListener() {
        @Override
        public void onDataChange( DataSnapshot snapshot ) {
            String value = "got it";
            future.set( value );
        }

        @Override
        public void onCancelled( DatabaseError error ) {
            future.set( null );
        }
    } );

    String value = null;
    try {
        value = future.get( 10, TimeUnit.SECONDS );
    } catch( InterruptedException e ) {
        log.error( "getValue interrupted: {}", e );
    } catch( ExecutionException e ) {
        log.error( "getValue did not execute: {}", e );
    } catch( TimeoutException e ) {
        log.error( "getValue timed-out: {}", e );
    }

from firebase-admin-java.

I believe that firebase is preventing my appengine basic scaling instances to be evicted. It seems that as long as a background thread is alive, instance will also be alive and won't respect the idle-timeout setting.

Also, every 12 hours I can see a crash on logs while appengine calls /_ah/background with bellow trace:

java.lang.RuntimeException: Restarting Firebase Worker Thread
	at com.google.firebase.internal.RevivingScheduledExecutor.<clinit>(RevivingScheduledExecutor.java:43)
	at com.google.firebase.internal.GaeScheduledExecutorService$ExecutorWrapper.<init>(GaeScheduledExecutorService.java:187)
	at com.google.firebase.internal.GaeScheduledExecutorService.ensureExecutorWrapper(GaeScheduledExecutorService.java:61)
	at com.google.firebase.internal.GaeScheduledExecutorService.ensureExecutorService(GaeScheduledExecutorService.java:70)
	at com.google.firebase.internal.GaeScheduledExecutorService.execute(GaeScheduledExecutorService.java:176)
	at com.google.firebase.tasks.ContinueWithCompletionListener.onComplete(ContinueWithCompletionListener.java:43)
	at com.google.firebase.tasks.TaskCompletionListenerQueue.flush(TaskCompletionListenerQueue.java:81)
	at com.google.firebase.tasks.TaskImpl.flushIfComplete(TaskImpl.java:242)
	at com.google.firebase.tasks.TaskImpl.continueWith(TaskImpl.java:176)
	at com.google.firebase.tasks.TaskImpl.continueWith(TaskImpl.java:166)
	at com.google.firebase.auth.FirebaseAuth.verifyIdToken(FirebaseAuth.java:186)
	at io.xxx.micro.services.FirebaseTokenServiceImpl.loadPrincipalIdFromIdToken(FirebaseTokenServiceImpl.java:51)

Please take this into account while refactoring the threading model.

from firebase-admin-java.

I've just confirmed what I said on my above comment. This link also gives more information about the issue.

I only use firebase auth to verify custom Id token across more than 30 micro services on appengine. They are all configured to be turned off if idle for 5 min but because firebase keeps the BG thread running for min 12h in a roll (doing nothing), instances are not evicted and costs increased drastically with no traffic at all.

Unfortunately I've had to remove firebase SDK from my sources and Verify ID tokens using a third-party JWT library.

from firebase-admin-java.

This is due to the proactive token refresh task, which runs in the background renewing OAuth2 tokens once every hour. This is scheduled on a thread pool, which in the current design cannot be shutdown cleanly. The problem is definitely in my radar, and I intend to fix it.

@nilsonfreitas I believe your situation is somewhat complicated though. I suppose you call FirebaseApp.initializeApp() once, after which your application stays alive until the instance becomes eligible for termination. Do you ever call app.delete() in your app? My current plan is to redesign the SDK so that a call to app.delete() will clean up any and all resources, including the background thread pools. But if you cannot call app.delete(), then we should also look into providing something like FirebaseOptions.Builder.setDisableProactiveTokenRefresh(boolean).

from firebase-admin-java.

@hiranya911, I call FirebaseApp.initializeApp() as soon as the first request arrives to that instance and never call app.delete().

Calling app.delete() doesn't make much sense considering GAE's architecture because instances don't know when they will be terminated. For example, while using basic scaling with idle-timeout = 5 minutes, GAE will terminate the instance if:

• No requests are processed on the last 5 minutes
• No BG threads are running

While redesigning the threading model for GAE we need to take into account the 3 scaling types. For example:

1. If running on basic scaling, no idle BG thread should be kept alive longer than idle-timeout. This is what increases costs even when there is no traffic.
2. If running on manual scaling they can be kept alive up to BG maximum thread lifetime. AFAIK is 24h.
3. If running on auto scaling thee is no BG thread at all.

Current implementations deals well for auto and manual scaling but not for basic.

from firebase-admin-java.

I am getting a similar error to @nilsonfreitas every 24 hours (I think) as follows:

Uncaught exception from servlet java.lang.RuntimeException: Restarting Firebase Worker Thread at com.google.firebase.internal.RevivingScheduledExecutor.<clinit>(RevivingScheduledExecutor.java:45) at com.google.firebase.database.core.GaePlatform.newEventTarget(GaePlatform.java:85) at com.google.firebase.database.core.Context.ensureEventTarget(Context.java:250) at com.google.firebase.database.core.Context.initServices(Context.java:120) at com.google.firebase.database.core.Context.freeze(Context.java:102) at com.google.firebase.database.core.RepoManager.createLocalRepo(RepoManager.java:95) at com.google.firebase.database.core.RepoManager.createRepo(RepoManager.java:49) at com.google.firebase.database.FirebaseDatabase.ensureRepo(FirebaseDatabase.java:337) at com.google.firebase.database.FirebaseDatabase.getReference(FirebaseDatabase.java:195) at uk.org.hassell.trafficalarm.DBListener.<init>(DBListener.java:62) at uk.org.hassell.trafficalarm.Startup.contextInitialized(Startup.java:29) at org.mortbay.jetty.handler.ContextHandler.startContext(ContextHandler.java:548) at org.mortbay.jetty.servlet.Context.startContext(Context.java:136) at org.mortbay.jetty.webapp.WebAppContext.startContext(WebAppContext.java:1250) at org.mortbay.jetty.handler.ContextHandler.doStart(ContextHandler.java:517) at org.mortbay.jetty.webapp.WebAppContext.doStart(WebAppContext.java:467) at org.mortbay.component.AbstractLifeCycle.start(AbstractLifeCycle.java:50) at com.google.apphosting.runtime.jetty.AppVersionHandlerMap.createHandler(AppVersionHandlerMap.java:203) at com.google.apphosting.runtime.jetty.AppVersionHandlerMap.getHandler(AppVersionHandlerMap.java:176) at com.google.apphosting.runtime.jetty.JettyServletEngineAdapter.serviceRequest(JettyServletEngineAdapter.java:134) at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchServletRequest(JavaRuntime.java:650) at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchRequest(JavaRuntime.java:612) at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.run(JavaRuntime.java:582) at com.google.tracing.TraceContext$TraceContextRunnable.runInContext(TraceContext.java:454) at com.google.tracing.TraceContext$TraceContextRunnable$1.run(TraceContext.java:461) at com.google.tracing.CurrentContext.runInContext(CurrentContext.java:297) at com.google.tracing.TraceContext$AbstractTraceContextCallback.runInInheritedContextNoUnref(TraceContext.java:320) at com.google.tracing.TraceContext$AbstractTraceContextCallback.runInInheritedContext(TraceContext.java:312) at com.google.tracing.TraceContext$TraceContextRunnable.run(TraceContext.java:458) at com.google.apphosting.runtime.ThreadGroupPool$PoolEntry.run(ThreadGroupPool.java:263) at java.lang.Thread.run(Thread.java:745)

In my case I am setting a listener for a Firebase Database, which I want to run indefinitely (and to carry out actions if the data changes). I was using the standard firebase server sdk, but that stopped completely after 24 hours, which is why I have switched to the firebase admin sdk. Is this possible?

from firebase-admin-java.

The error is to be expected when running on App Engine. The SDK restarts threads every 12 hours or so, to prevent them from being killed by the GAE runtime. Your app should continue to work regardless of the error message.

from firebase-admin-java.

@nilsonfreitas I'm in the process of implementing a number of improvements to the thread model of the SDK. However, it is pretty much impossible to build a unified model that will work equally well in the regular JVM, and 3 different scaling modes of App Engine. This is made even more complicated by the fact that when running in GAE there is no way to reliably determine which scaling mode is in use. Therefore here's what the final outcome is going to look like:

1. The SDK will work well in the regular JVM. We can ensure graceful termination of thread pools and the SDK, even if the user ignores to call app.delete().
2. The SDK will work well in GAE with manual and auto scaling. Database will not work with auto scaling since it requires background thread support. With manual scaling, app.delete() will ensure termination of all threads if that kind of clean up is necessary.
3. With basic scaling and default settings in the SDK, a call to app.delete() will be required to terminate threads (by default SDK starts threads using the background thread factory). However, the new ThreadManager interface will allow developers to use a request-scoped thread factory or a "direct" executor when deploying with basic scaling (note that database cannot be accessed when used this way). This will guarantee instance tear down after idle period, which should solve the problem you've stated above.
4. In future we will also introduce some blocking methods to interfaces like FirebaseAuth which should take thread management concerns away from the users who are running simple operations like verifyIdToken() in GAE.

I will update this issue when the code is ready, so perhaps you can run some tests and give us some feedback.

from firebase-admin-java.

@hiranya911 , yes I can run some tests once done.

I was checking GaeScheduledExecutorService.java and RevivingScheduledExecutor.java and I have some suggestions about them.

When running on basic scaling the Reviving approach doesn't make much sense because we want the thread to be killed and not restarted after idle timeout. We also don't desire the default ThreadPoolExecutor because it is suitable only when no BG threads aren't supported.

If you also make the ExecutorService pluggable, we could create a third implementation just for basic scaling that can be configured to keep threads alive for a maximum idle timeout. For example:

new ThreadPoolExecutor(
    1, // Or as many core threads as app needs
    10, // Max allowed BG threads or any value bellow it
    10L, // Keep them alive considering idle timeout
    TimeUnit.MINUTES,
    new SynchronousQueue<Runnable>(),
    threadFactory);

Lets say that I need 30 min idle timeout on GAE. A possible configuration to keep the 30 min and also have BG threads is:

• Setup 05min idle timeout for GAE scaling
• Setup 25min timeout while initializing ThreadPoolExecutor

This third implementation doesn't need to be part of SDK and it is up do developers to customize it accordingly. This will only possible by making ExecutorService pluggable.

Thanks

from firebase-admin-java.

@nilsonfreitas The ThreadManager interface we are about to introduce will make it possible for developers to plug in their own executor services and thread factories.

from firebase-admin-java.

All changes except for the blocking methods have been implemented in https://firebase.google.com/support/release-notes/admin/java#5.4.0

• Task API is now deprecated.
• ThreadManager API supports configuring thread pool and factory used by the SDK.
• GAE basic scaling has been tested with a custom ThreadManager implementation.

Blocking methods to complement the new *Async() methods will be introduced in a future release.

from firebase-admin-java.

@hiranya911 , thanks for the update. I've been using 5.5.0 in production and all looks fine. I have just one small concern about bellow log message:

Falling back to GAE's request-scoped threads. Firebase requires manually-scaled instances for most operations

It is currently being logged as a warning message here.

On the way as I see, warning means "we are fine for the time being but please take an action so I will stop warning you". =)

On my projects, I also use firebase auth for auto scaling instances and BG threads will never be available. And I also don't need BG threads at all because I don't have real time DB.

Please consider making this log an info message instead of warning because it can be part of regular and valid use cases.

from firebase-admin-java.

@nilsonpontello Good point. I'll make the change. Thanks for pointing this out.

from firebase-admin-java.

@hiranya911 I'm having a hard time figuring out how to use ApiFuture.

I've found some articles explaining the usage of the (now deprecated) Tasks but can't find anything useful for ApiFuture apart from the Java docs. I also looked at the firebase-admin-api tests but those cover only getting a future and waiting for it. I know ApiFuture is a separate thing, but some nice examples would probably increase adoption. Here's a nice article explaining the usage of Tasks with Firebase on SO: Call Firebase database from Google App Engine.

Keep up the good work.

from firebase-admin-java.

@samodadela I would recommend you to take a look at the ApiFutures helper class, which provides some utilities for dealing with ApiFuture instances. I'm also working on an article with more detailed examples. I'll post a link here when it goes live (hopefully by early next week).

from firebase-admin-java.

@hiranya911 I looked at the helper class but there's not a single example or description. I also checked the tests for ApiFutures but I still don't get it. It's a bit difficult to know what the designer of the class meant and what the usages are. I see there's a connection with Guava futures so I'll try to understand that first - otherwise I'll just stick with Tasks awaiting for your article.

Thanks.

from firebase-admin-java.

@samodadela Thanks for sharing your code snippet. Only thing I would change is your future.set(null) statement in the onCancelled() event handler. I would do future.setException(error.toException()) instead.

from firebase-admin-java.

I've posted an article about migrating from Task to ApiFuture at https://medium.com/@hiranya911/firebase-asynchronous-operations-with-admin-java-sdk-82ca9b4f6022

from firebase-admin-java.

@hiranya911 thank you very much... nice article!

from firebase-admin-java.

Hi @hiranya911 , have the *Async calls on the server side is great, but getting rid of the Task based one is a big problem, because we can no longer share code between app and server.
In our case, we are actually using the app code on the server side to automate our tests, which is quite useful and powerful.
This would no longer be possible if we nuke the Task based interface.

from firebase-admin-java.

I'm getting this message still, even after moving from Task to the Async operations in Firebase Admin

Falling back to GAE's request-scoped threads. Firebase requires manually-scaled instances for most operations.

I prefer to continue using AppEngine Standard. Is it safe to continue with Standard?

from firebase-admin-java.

@washowasho Can't use FirebaseDatabase on GAE standard (at least not without a bit of custom code). Everything else should work out fine.

from firebase-admin-java.

@hiranya911 Thanks. I'm only using it for Auth, so I guess I am good.

Should I just ignore the error I previously mentioned? Is that an issue that's being worked on? Or should I be doing something different?

Thanks.

from firebase-admin-java.

You can ignore it since it's an info-level log (not an error). It will also go away in a future release, once the GAE support refactor in the v6 branch goes into effect. You can also get rid of it now by providing a custom ThreadManager implementation, but I don't think it's worth the effort.

from firebase-admin-java.

@washowasho Can't use FirebaseDatabase on GAE standard (at least not without a bit of custom code). Everything else should work out fine.

@hiranya911 I am using this library to access my Firebase Database on GAE Standard. It has been working well until recently, when I have been getting lots of errors like this: "The process handling this request unexpectedly died. This is likely to cause a new process to be used for the next request to your application. (Error code 203)" My app is simply looking for changes to the database, and this error occurs after about 30 minutes of no activity. Do I need to use some custom code?

P.S. My app is built around Push Queues, so I don't think I can switch to GAE Flexible without a lot of pain.
Thanks.

from firebase-admin-java.

@engeeaitch @washowasho Let me rephrase. The GAE scaling mode is more important to the SDK than the GAE environment. GAE standard will work for RTDB as long as you're using manual scaling. Other scaling modes (auto and basic) require some custom code.

from firebase-admin-java.

@hiranya911 Thanks - I'm using manual scaling, so should be OK (but I still don't know what is causing my 203 errors).

from firebase-admin-java.

@engeeaitch Try reaching out to App Engine support? Please update this thread if you find out anything useful.

from firebase-admin-java.

I will close this issue since the API refactor is now complete and released. There's one incorrectly named blocking method in FirebaseAuth API (setCustomClaims()), which will be fixed in the next minor release (see #175).

from firebase-admin-java.

For what it's worth, I reported the 203 errors to Google, who eventually 'fixed' the problem - there was no real explanation other than "an internal issue with regards to logging in Java where it uses ioctl(SIOCGIFCONF) and throwing exception when it fails. This may affect several Java projects and your application can be one of it." I did not need to make any changes to my app.

from firebase-admin-java.