[keycloak-user] ProviderFactory::postInit + transactions = startup failure

[Dmitry Telegin]

On Mon, 03/07/2017 13:43 +0200, Marek Posolda wrote:
> On 03/07/17 13:01, Dmitry Telegin wrote:
> > Hi Marek,
> > 
> > Thanks for the hint, looks very reasonable. I think the
> > PostMigrationEvent approach is more suitable for heavyweight stuff
> > like checking and creating database entries etc., while lazyInit is
> > good for more lightweight tasks like opening connections etc.
> > 
> > However, there's an interesting case when the postInit method is
> > called on hot (re)deploy (no PostMigrationEvent obviously) *and*
> > some heavyweight stuff needs to be done right away. (ATM postInit
> > is not called on hot deploy, but I hope that will be fixed soon,
> > see KEYCLOAK-5131 and PR #4282.)
> > Luckily, in this case transactions just work, so everything could
> > be done straightforwardly. The only question is how to distinguish
> > between server startup and hot (re)deploy inside postInit. There
> > are some indirect signs like thread name, presence/absence of
> > specific JNDI entries etc., but this seems hacky. Any suggestions?
>  I don't have any good suggestions besides other workaround. You can
> create provider, which will be deployed "statically" and will track
> whether PostMigrationEvent was already sent. Since it is deployed at
> startup like builtin providers, the event will be always there. The
> "dynamic" providers will be then able either to ask this provider or
> listen to the event.

Another (simpler) approach is to query Resteasy for the
presence/absence of Keycloak-specific classes, because Resteasy
deployment happens right after PostMigrateEvent, but this seems very
hacky too :-\

> 
> Maybe we need to have another lifecycle method on ProviderFactory for
> this usecase, not sure..

How about ProviderFactory::postInit(KeycloakSessionFactory factory,
boolean hot) with default implementation delegating to
postInit(KeycloakSessionFactory factory)?

Dmitry

> 
> Marek
> > Cheers,
> > Dmitry
> > 
> > > Hi,
> > > 
> > > I think it's not good to directly start transactions from
> > > postInit. 
> > > Among the issues you mentioned, various initial steps (eg.
> > > migration 
> > > from previous version, export/import) may not be yet finished at
> > > this 
> > > stage. Probably you can either:
> > > - Register listener for PostMigrationEvent in your postInit. See
> > > the 
> > > testsuite/integration-arquillian/servers/auth-
> > > server/services/testsuite-
> > > providers/src/main/java/org/keycloak/testsuite/authentication/Pus
> > > hButtonAuthenticator.java 
> > > for inspiration.
> > > - Use the pattern with "lazyInit" like for example here 
> > > https://github.com/keycloak/keycloak/blob/master/model/jpa/src/ma
> > > in/java/org/keycloak/connections/jpa/DefaultJpaConnectionProvider
> > > Factory.java#L78 
> > > , which is called 1st time before your provider is actually
> > > needed.
> > > 
> > > Maybe we can improve by provide more callback methods to 
> > > ProviderFactory/Provider to avoid the lazyInit pattern directly
> > > in 
> > > providers, but rather integrate it better with the Provider
> > > framework. 
> > > But for now, I think that some of the workaround above should
> > > work for 
> > > you IMO.
> > > 
> > > Marek
> > > 
> > > On 03/07/17 03:50, Dmitry Telegin wrote:
> > > > https://issues.jboss.org/browse/KEYCLOAK-5132
> > > > 
> > > > Meanwhile I've found a workaround - just run a transaction in a
> > > > new
> > > > thread. However, this should be a "managed thread" - see issue
> > > > details
> > > > & comments for more info.
> > > > 
> > > > В Wed, 28/06/2017 в 21:18 +0300, Dmitry Telegin пишет:
> > > > > Hi,
> > > > > 
> > > > > (TL;DR) if a KeycloakTransaction is opened from
> > > > > ProviderFactory::postInit, sometimes the transaction is
> > > > > already
> > > > > active
> > > > > on the underlying
> > > > > org.jboss.jca.adapters.jdbc.local.LocalManagedConnection,
> > > > > which leads
> > > > > to errors.
> > > > > 
> > > > > (full version) I think it's essential for the providers to be
> > > > > able to
> > > > > access realm data in postInit(). For that, a transaction is
> > > > > required;
> > > > > using KeycloakModelUtils.runJobInTransaction() is a
> > > > > convenient method
> > > > > to do that:
> > > > > 
> > > > >      @Override
> > > > >      public void postInit(KeycloakSessionFactory factory) {
> > > > >          KeycloakModelUtils.runJobInTransaction(factory,
> > > > > (KeycloakSession session) -> {
> > > > >              List<RealmModel> realms =
> > > > > session.realms().getRealms();
> > > > >              // do stuff
> > > > >          });
> > > > >      }
> > > > > 
> > > > > When such a provider is deployed, in about half of cases
> > > > > Keycloak
> > > > > fails
> > > > > to start due to the following exception:
> > > > > 
> > > > > java.sql.SQLException: IJ031017: You cannot set autocommit
> > > > > during a
> > > > > managed transaction
> > > > > 
> > > > > (see full stacktrace here https://pastebin.com/ETtPqXQk)
> > > > > 
> > > > > I've managed to track it down to something that looks like
> > > > > transaction
> > > > > clash over a single instance of
> > > > > org.jboss.jca.adapters.jdbc.local.LocalManagedConnection.
> > > > > What
> > > > > happens
> > > > > is that the two treads at the same time begin two
> > > > > KeycloakTransactions
> > > > > which end up with the same instance of
> > > > > LocalManagedConnection. The
> > > > > above exception results from the second begin() call.
> > > > > 
> > > > > There's a system property called
> > > > > "ironjacamar.jdbc.ignoreautocommit"
> > > > > that allows to ignore the situation, but I think it's
> > > > > dangerous
> > > > > because
> > > > > it doesn't eliminate the transaction clash, just suppresses
> > > > > the
> > > > > check.
> > > > > If I'm not mistaken, this began to happen around Keycloak
> > > > > 2.2.x,
> > > > > which
> > > > > coincides with the changes to Keycloak transaction
> > > > > management. That
> > > > > said, do I need now some additional transaction coordination
> > > > > with the
> > > > > rest of Keycloak, or is it a bug? If former, how do I do
> > > > > that? If
> > > > > latter, how do we fix it?
> > > > > 
> > > > > I hope we'll sort it out, since the ability to access the
> > > > > data at
> > > > > every
> > > > > phase of provider's lifecycle seems something fundamental to
> > > > > me.
> > > > > 
> > > > > Regards,
> > > > > Dmitry
> > > > > _______________________________________________
> > > > > keycloak-user mailing list
> > > > > keycloak-user at lists.jboss.org
> > > > > https://lists.jboss.org/mailman/listinfo/keycloak-user
> > > > 
> > > > _______________________________________________
> > > > keycloak-user mailing list
> > > > keycloak-user at lists.jboss.org
> > > > https://lists.jboss.org/mailman/listinfo/keycloak-user
> > > 
> > > 
> 
>