From 22e4e61ca341db2bb5d84852234d144fa7641eec Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Thu, 12 Jan 2023 11:19:00 +0100 Subject: [PATCH] Prevent endless loop in case of split-brain Fixes: #16427 --- ...ltInfinispanConnectionProviderFactory.java | 1 + ...nispanUserLoginFailureProviderFactory.java | 9 ++++- .../InfinispanUserSessionProviderFactory.java | 13 ++++++- .../initializer/BaseCacheInitializer.java | 5 +++ .../initializer/CacheInitializer.java | 37 ++++++++++++++++++- .../DBLockBasedCacheInitializer.java | 9 +++++ .../InfinispanCacheInitializer.java | 9 ++++- .../initializer/InitializerState.java | 5 +++ .../model/parameters/Infinispan.java | 11 +++++- 9 files changed, 93 insertions(+), 6 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/connections/infinispan/DefaultInfinispanConnectionProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/connections/infinispan/DefaultInfinispanConnectionProviderFactory.java index f60a9eec5a2..c2e6dea88da 100755 --- a/model/infinispan/src/main/java/org/keycloak/connections/infinispan/DefaultInfinispanConnectionProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/connections/infinispan/DefaultInfinispanConnectionProviderFactory.java @@ -259,6 +259,7 @@ public class DefaultInfinispanConnectionProviderFactory implements InfinispanCon .l1() .enabled(l1Enabled) .lifespan(l1Lifespan) + .stateTransfer().timeout(30, TimeUnit.SECONDS) .build(); } diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserLoginFailureProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserLoginFailureProviderFactory.java index b25d961b5b4..ff177a2f853 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserLoginFailureProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserLoginFailureProviderFactory.java @@ -181,6 +181,10 @@ public class InfinispanUserLoginFailureProviderFactory implements UserLoginFailu } } + private int getStalledTimeoutInSeconds(int defaultTimeout) { + return config.getInt("stalledTimeoutInSeconds", defaultTimeout); + } + private void loadLoginFailuresFromRemoteCaches(final KeycloakSessionFactory sessionFactory, String cacheName, final int sessionsPerSegment, final int maxErrors) { log.debugf("Check pre-loading sessions from remote cache '%s'", cacheName); @@ -190,9 +194,12 @@ public class InfinispanUserLoginFailureProviderFactory implements UserLoginFailu public void run(KeycloakSession session) { InfinispanConnectionProvider connections = session.getProvider(InfinispanConnectionProvider.class); Cache workCache = connections.getCache(InfinispanConnectionProvider.WORK_CACHE_NAME); + int defaultStateTransferTimeout = (int) (connections.getCache(InfinispanConnectionProvider.LOGIN_FAILURE_CACHE_NAME) + .getCacheConfiguration().clustering().stateTransfer().timeout() / 1000); InfinispanCacheInitializer initializer = new InfinispanCacheInitializer(sessionFactory, workCache, - new RemoteCacheSessionsLoader(cacheName, sessionsPerSegment), "remoteCacheLoad::" + cacheName, sessionsPerSegment, maxErrors); + new RemoteCacheSessionsLoader(cacheName, sessionsPerSegment), "remoteCacheLoad::" + cacheName, sessionsPerSegment, maxErrors, + getStalledTimeoutInSeconds(defaultStateTransferTimeout)); initializer.initCache(); initializer.loadSessions(); diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java index 3d1d37c5a00..536a2f243d2 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java @@ -165,6 +165,9 @@ public class InfinispanUserSessionProviderFactory implements UserSessionProvider return timeout != null ? timeout : Environment.getServerStartupTimeout(); } + private int getStalledTimeoutInSeconds(int defaultTimeout) { + return config.getInt("sessionPreloadStalledTimeoutInSeconds", defaultTimeout); + } @Override public void loadPersistentSessions(final KeycloakSessionFactory sessionFactory, final int maxErrors, final int sessionsPerSegment) { @@ -180,9 +183,12 @@ public class InfinispanUserSessionProviderFactory implements UserSessionProvider InfinispanConnectionProvider connections = session.getProvider(InfinispanConnectionProvider.class); Cache workCache = connections.getCache(InfinispanConnectionProvider.WORK_CACHE_NAME); + int defaultStateTransferTimeout = (int) (connections.getCache(InfinispanConnectionProvider.OFFLINE_USER_SESSION_CACHE_NAME) + .getCacheConfiguration().clustering().stateTransfer().timeout() / 1000); InfinispanCacheInitializer ispnInitializer = new InfinispanCacheInitializer(sessionFactory, workCache, - new OfflinePersistentUserSessionLoader(sessionsPerSegment), "offlineUserSessions", sessionsPerSegment, maxErrors); + new OfflinePersistentUserSessionLoader(sessionsPerSegment), "offlineUserSessions", sessionsPerSegment, maxErrors, + getStalledTimeoutInSeconds(defaultStateTransferTimeout)); // DB-lock to ensure that persistent sessions are loaded from DB just on one DC. The other DCs will load them from remote cache. CacheInitializer initializer = new DBLockBasedCacheInitializer(session, ispnInitializer); @@ -326,9 +332,12 @@ public class InfinispanUserSessionProviderFactory implements UserSessionProvider public void run(KeycloakSession session) { InfinispanConnectionProvider connections = session.getProvider(InfinispanConnectionProvider.class); Cache workCache = connections.getCache(InfinispanConnectionProvider.WORK_CACHE_NAME); + int defaultStateTransferTimeout = (int) (connections.getCache(InfinispanConnectionProvider.USER_SESSION_CACHE_NAME) + .getCacheConfiguration().clustering().stateTransfer().timeout() / 1000); InfinispanCacheInitializer initializer = new InfinispanCacheInitializer(sessionFactory, workCache, - new RemoteCacheSessionsLoader(cacheName, sessionsPerSegment), "remoteCacheLoad::" + cacheName, sessionsPerSegment, maxErrors); + new RemoteCacheSessionsLoader(cacheName, sessionsPerSegment), "remoteCacheLoad::" + cacheName, sessionsPerSegment, maxErrors, + getStalledTimeoutInSeconds(defaultStateTransferTimeout)); initializer.initCache(); initializer.loadSessions(); diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/BaseCacheInitializer.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/BaseCacheInitializer.java index 5241eb5feea..196b4cfddf3 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/BaseCacheInitializer.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/BaseCacheInitializer.java @@ -72,6 +72,11 @@ public abstract class BaseCacheInitializer extends CacheInitializer { return transport == null || transport.isCoordinator(); } + @Override + protected int getProgressIndicator() { + InitializerState state = getStateFromCache(); + return state == null ? 0 : state.getProgressIndicator(); + } protected InitializerState getStateFromCache() { // We ignore cacheStore for now, so that in Cross-DC scenario (with RemoteStore enabled) is the remoteStore ignored. diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/CacheInitializer.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/CacheInitializer.java index 8740d7bf698..725005371fe 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/CacheInitializer.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/CacheInitializer.java @@ -17,6 +17,8 @@ package org.keycloak.models.sessions.infinispan.initializer; +import java.time.Instant; +import java.util.concurrent.TimeUnit; import org.jboss.logging.Logger; /** @@ -30,10 +32,32 @@ public abstract class CacheInitializer { } public void loadSessions() { + Instant loadingMustContinueBy = Instant.now().plusSeconds(getStalledTimeoutInSeconds()); + boolean loadingStalledInPreviousStep = false; + int lastProgressIndicator = 0; while (!isFinished()) { if (!isCoordinator()) { try { - Thread.sleep(1000); + TimeUnit.SECONDS.sleep(1); + + final int progressIndicator = getProgressIndicator(); + final boolean loadingStalled = lastProgressIndicator == progressIndicator; + if (loadingStalled) { + if (loadingStalledInPreviousStep) { + if (Instant.now().isAfter(loadingMustContinueBy)) { + throw new RuntimeException("Loading sessions has stalled for " + getStalledTimeoutInSeconds() + " seconds, possibly caused by split-brain"); + } + + log.tracef("Loading sessions stalled. Waiting until %s", loadingMustContinueBy); + } else { + loadingMustContinueBy = Instant.now().plusSeconds(getStalledTimeoutInSeconds()); + loadingStalledInPreviousStep = true; + } + } else { + loadingStalledInPreviousStep = false; + } + + lastProgressIndicator = progressIndicator; } catch (InterruptedException ie) { log.error("Interrupted", ie); throw new RuntimeException("Loading sessions failed", ie); @@ -49,8 +73,19 @@ public abstract class CacheInitializer { protected abstract boolean isCoordinator(); + /** + * Returns an integer which captures current progress. If there is a progress in loading, + * this indicator must be different most of the time so that it does not hit 30-seconds + * limit. + * @see #stalledTimeoutInSeconds + * @return + */ + protected abstract int getProgressIndicator(); + /** * Just coordinator will run this */ protected abstract void startLoading(); + + protected abstract int getStalledTimeoutInSeconds(); } diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/DBLockBasedCacheInitializer.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/DBLockBasedCacheInitializer.java index c491a48c50e..943e1d05647 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/DBLockBasedCacheInitializer.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/DBLockBasedCacheInitializer.java @@ -57,6 +57,15 @@ public class DBLockBasedCacheInitializer extends CacheInitializer { return delegate.isCoordinator(); } + @Override + protected int getProgressIndicator() { + return delegate.getProgressIndicator(); + } + + @Override + protected int getStalledTimeoutInSeconds() { + return delegate.getStalledTimeoutInSeconds(); + } /** * Just coordinator will run this. And there is DB-lock, so the delegate.startLoading() will be permitted just by the single DC diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InfinispanCacheInitializer.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InfinispanCacheInitializer.java index 26df7ea5f95..f7369e5b099 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InfinispanCacheInitializer.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InfinispanCacheInitializer.java @@ -51,10 +51,13 @@ public class InfinispanCacheInitializer extends BaseCacheInitializer { private final int maxErrors; + // Effectively no timeout + private final int stalledTimeoutInSeconds; - public InfinispanCacheInitializer(KeycloakSessionFactory sessionFactory, Cache workCache, SessionLoader sessionLoader, String stateKeySuffix, int sessionsPerSegment, int maxErrors) { + public InfinispanCacheInitializer(KeycloakSessionFactory sessionFactory, Cache workCache, SessionLoader sessionLoader, String stateKeySuffix, int sessionsPerSegment, int maxErrors, int stalledTimeoutInSeconds) { super(sessionFactory, workCache, sessionLoader, stateKeySuffix, sessionsPerSegment); this.maxErrors = maxErrors; + this.stalledTimeoutInSeconds = stalledTimeoutInSeconds; } @@ -111,6 +114,10 @@ public class InfinispanCacheInitializer extends BaseCacheInitializer { startLoadingImpl(state, ctx[0]); } + @Override + protected int getStalledTimeoutInSeconds() { + return this.stalledTimeoutInSeconds; + } protected void startLoadingImpl(InitializerState state, SessionLoader.LoaderContext loaderCtx) { // Assume each worker has same processor's count diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InitializerState.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InitializerState.java index d5ed5ff99f5..3edfdb31690 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InitializerState.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InitializerState.java @@ -73,6 +73,11 @@ public class InitializerState extends SessionEntity { return segments.cardinality() == segmentsCount; } + /** Return indication of progress - changes upon progress */ + public int getProgressIndicator() { + return segments.hashCode(); + } + /** Return next un-finished segments in the next row of segments. * @param segmentToLoad The segment we are loading * @param maxSegmentCount The max segment to load diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/Infinispan.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/Infinispan.java index 40c4a0d39d6..ebdd303db20 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/Infinispan.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/Infinispan.java @@ -23,6 +23,8 @@ import org.keycloak.keys.PublicKeyStorageSpi; import org.keycloak.keys.infinispan.InfinispanCachePublicKeyProviderFactory; import org.keycloak.keys.infinispan.InfinispanPublicKeyStorageProviderFactory; import org.keycloak.models.SingleUseObjectSpi; +import org.keycloak.models.UserLoginFailureSpi; +import org.keycloak.models.UserSessionSpi; import org.keycloak.models.cache.authorization.CachedStoreFactorySpi; import org.keycloak.models.cache.infinispan.authorization.InfinispanCacheStoreFactoryProviderFactory; import org.keycloak.models.cache.CachePublicKeyProviderSpi; @@ -98,7 +100,14 @@ public class Infinispan extends KeycloakModelParameters { .config("embedded", "true") .config("clustered", "true") .config("useKeycloakTimeService", "true") - .config("nodeName", "node-" + NODE_COUNTER.incrementAndGet()); + .config("nodeName", "node-" + NODE_COUNTER.incrementAndGet()) + .spi(UserLoginFailureSpi.NAME) + .provider(InfinispanUserLoginFailureProviderFactory.PROVIDER_ID) + .config("stalledTimeoutInSeconds", "10") + .spi(UserSessionSpi.NAME) + .provider(InfinispanUserSessionProviderFactory.PROVIDER_ID) + .config("sessionPreloadStalledTimeoutInSeconds", "10") + ; } public Infinispan() {