[keycloak-user] ProviderFactory::postInit + transactions = startup failure
Dmitry Telegin
mitya at cargosoft.ru
Mon Jul 3 18:34:16 EDT 2017
Hi Bill, thanks for the suggestion,
> I think the problem is that
> DefaultJpaConnectionProviderFactory.checkJtaEnabled() is called
> after
> your provider's postInit method. The error you are seeing is that
> JPA
> is trying to use a JDBC connection features within a JTA transaction
> that it is not supposed to.
>
> I think you might be able to call checkJtaEnabled within
> DefaultJpaConnectionProviderFactory.lazyInit and that will solve the
> problem.
I think you meant DefaultJpaConnectionProviderFactory.postInit? (since
lazyInit is private and doesn't call checkJtaEnabled)
Indeed, I've added the following lines to my postInit:
ProviderFactory<JpaConnectionProvider> jpa =
factory.getProviderFactory(JpaConnectionProvider.class);
jpa.postInit(factory);
// now do the real stuff
KeycloakModelUtils.runJobInTransaction(factory,
(KeycloakSession session) -> { ... }
...and this worked, the result is very stable, no exceptions are
thrown. However, as Marek has noted, postInit is called really very
early. For example, on the very first run it is invoked even before
database schema is created and master realm is initialized, so it ends
up with an empty list of realms.
To sum up, we have the following three approaches:
1. if absolutely needed, transactions can be used in postInit right
away after the explicit call to
DefaultJpaConnectionProviderFactory.postInit(). Should be used at one's
risk since it is called early;
2. listen for PostMigrationEvent;
3. implement lazyInit pattern.
If this situation with checkJtaEnabled() is kind of by design, I think
KEYCLOAK-5132 can be closed w/o resolution, provided that we document
the above approaches somewhere.
Thanks again for your help!
Dmitry
>
>
> On 7/3/17 10:43 AM, Marek Posolda wrote:
> > On 03/07/17 15:05, Dmitry Telegin wrote:
> > > 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)?
> >
> > Not sure TBH.
> >
> > I personally don't know how the deployer lifecycle works and if
> > ProviderFactory.postInit for the "deployer" providers still can't
> > be
> > called earlier than auth-server is fully bootstrapped. Also you
> > would
> > need some "if" checks in the postInit method to differentiate
> > between
> > both cases, which doesn't look so great to me personally.
> >
> > Having new method should be good from backwards compatibility
> > perspective, as we have JDK8 and you can easily add new empty
> > "default"
> > method to the ProviderFactory interface without affect existing
> > providers, which already use current "postInit" method with current
> > signature.
> >
> > Maybe will be good to move the discussion to keycloak-dev though.
> > Perhaps you can start the thread here?
> >
> > Thanks,
> > Marek
> > > 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/authenticati
> > > > > > on/PushButtonAuthenticator.java
> > > > > >
> > > > > > for inspiration.
> > > > > > - Use the pattern with "lazyInit" like for example here
> > > > > > https://github.com/keycloak/keycloak/blob/master/model/jpa/
> > > > > > src/main/java/org/keycloak/connections/jpa/DefaultJpaConnec
> > > > > > tionProviderFactory.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.LocalManagedConnectio
> > > > > > > > n, 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(factor
> > > > > > > > y,
> > > > > > > > (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.LocalManagedConnectio
> > > > > > > > n. 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 <mailto:keycloak-
> > > > > > > > user at lists.jboss.org>
> > > > > > > > https://lists.jboss.org/mailman/listinfo/keycloak-user
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > keycloak-user mailing list
> > > > > > > keycloak-user at lists.jboss.org <mailto: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
>
> _______________________________________________
> keycloak-user mailing list
> keycloak-user at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-user
More information about the keycloak-user
mailing list