diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/sessions/AbstractLastSessionRefreshStoreFactory.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/sessions/AbstractLastSessionRefreshStoreFactory.java index 3427ee3f07..ad0ea93c77 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/sessions/AbstractLastSessionRefreshStoreFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/sessions/AbstractLastSessionRefreshStoreFactory.java @@ -20,6 +20,7 @@ package org.keycloak.models.sessions.infinispan.changes.sessions; import org.keycloak.common.util.Time; import org.keycloak.models.KeycloakSession; import org.keycloak.models.utils.SessionTimeoutHelper; +import org.keycloak.timer.ScheduledTask; import org.keycloak.timer.TimerProvider; /** @@ -36,14 +37,22 @@ public abstract class AbstractLastSessionRefreshStoreFactory { // Max count of lastSessionRefreshes. If count of lastSessionRefreshes reach this value, the message is sent to second DC public static final int DEFAULT_MAX_COUNT = 100; - - protected void setupPeriodicTimer(KeycloakSession kcSession, AbstractLastSessionRefreshStore store, long timerIntervalMs, String eventKey) { TimerProvider timer = kcSession.getProvider(TimerProvider.class); - timer.scheduleTask((KeycloakSession keycloakSession) -> { + timer.scheduleTask(new PropagateLastSessionRefreshTask(store), timerIntervalMs, eventKey); + } - store.checkSendingMessage(keycloakSession, Time.currentTime()); + public static class PropagateLastSessionRefreshTask implements ScheduledTask { - }, timerIntervalMs, eventKey); + private final AbstractLastSessionRefreshStore store; + + public PropagateLastSessionRefreshTask(AbstractLastSessionRefreshStore store) { + this.store = store; + } + + @Override + public void run(KeycloakSession session) { + store.checkSendingMessage(session, Time.currentTime()); + } } } diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/DefaultJpaConnectionProviderFactory.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/DefaultJpaConnectionProviderFactory.java index c7ecf0e4ed..ba17fb9f04 100755 --- a/model/jpa/src/main/java/org/keycloak/connections/jpa/DefaultJpaConnectionProviderFactory.java +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/DefaultJpaConnectionProviderFactory.java @@ -342,7 +342,7 @@ public class DefaultJpaConnectionProviderFactory implements JpaConnectionProvide protected void startGlobalStats(KeycloakSession session, int globalStatsIntervalSecs) { logger.debugf("Started Hibernate statistics with the interval %s seconds", globalStatsIntervalSecs); TimerProvider timer = session.getProvider(TimerProvider.class); - timer.scheduleTask(new HibernateStatsReporter(emf), globalStatsIntervalSecs * 1000, "ReportHibernateGlobalStats"); + timer.scheduleTask(new HibernateStatsReporter(emf), globalStatsIntervalSecs * 1000); } void migration(MigrationStrategy strategy, boolean initializeEmpty, String schema, File databaseUpdateFile, Connection connection, KeycloakSession session) { diff --git a/model/storage-private/src/main/java/org/keycloak/services/scheduled/ClearExpiredAdminEvents.java b/model/storage-private/src/main/java/org/keycloak/services/scheduled/ClearExpiredAdminEvents.java index c161c77407..65b2fa7620 100755 --- a/model/storage-private/src/main/java/org/keycloak/services/scheduled/ClearExpiredAdminEvents.java +++ b/model/storage-private/src/main/java/org/keycloak/services/scheduled/ClearExpiredAdminEvents.java @@ -19,12 +19,10 @@ package org.keycloak.services.scheduled; import org.jboss.logging.Logger; import org.keycloak.common.util.Time; -import org.keycloak.events.EventStoreProvider; -import org.keycloak.events.EventStoreProviderFactory; import org.keycloak.models.KeycloakSession; -import org.keycloak.provider.InvalidationHandler; import org.keycloak.storage.datastore.PeriodicEventInvalidation; import org.keycloak.timer.ScheduledTask; + public class ClearExpiredAdminEvents implements ScheduledTask { protected static final Logger logger = Logger.getLogger(ClearExpiredAdminEvents.class); @@ -34,7 +32,7 @@ public class ClearExpiredAdminEvents implements ScheduledTask { long currentTimeMillis = Time.currentTimeMillis(); session.invalidate(PeriodicEventInvalidation.JPA_EVENT_STORE); long took = Time.currentTimeMillis() - currentTimeMillis; - logger.debugf("ClearExpiredEvents finished in %d ms", took); + logger.debugf("%s finished in %d ms", getTaskName(), took); } } diff --git a/model/storage-private/src/main/java/org/keycloak/services/scheduled/ClearExpiredUserSessions.java b/model/storage-private/src/main/java/org/keycloak/services/scheduled/ClearExpiredUserSessions.java index 904dc1f6e3..c7fa73dbee 100755 --- a/model/storage-private/src/main/java/org/keycloak/services/scheduled/ClearExpiredUserSessions.java +++ b/model/storage-private/src/main/java/org/keycloak/services/scheduled/ClearExpiredUserSessions.java @@ -39,7 +39,11 @@ public class ClearExpiredUserSessions implements ScheduledTask { session.sessions().removeAllExpired(); long took = Time.currentTimeMillis() - currentTimeMillis; - logger.debugf("ClearExpiredUserSessions finished in %d ms", took); + logger.debugf("%s finished in %d ms", getTaskName(), took); } + @Override + public String getTaskName() { + return TASK_NAME; + } } diff --git a/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java b/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java index e5cf78a66f..c6f21fcaa2 100755 --- a/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java +++ b/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java @@ -39,6 +39,7 @@ import org.keycloak.credential.CredentialAuthentication; import org.keycloak.credential.CredentialInput; import org.keycloak.credential.CredentialProvider; import org.keycloak.credential.CredentialProviderFactory; +import org.keycloak.models.AbstractKeycloakTransaction; import org.keycloak.models.ClientModel; import org.keycloak.models.ClientScopeModel; import org.keycloak.models.CredentialValidationOutput; @@ -789,7 +790,7 @@ public class UserStorageManager extends AbstractStorageManager getScheduledTasks() { + return Arrays.asList(new ClearExpiredEvents(), new ClearExpiredAdminEvents(), new ClearExpiredClientInitialAccessTokens(), new ClearExpiredUserSessions()); + } + + protected void scheduleTask(TimerProvider timer, KeycloakSessionFactory sessionFactory, ScheduledTask task, long interval) { + timer.schedule(new ClusterAwareScheduledTaskRunner(sessionFactory, task, interval), interval); + logger.debugf("Scheduled cluster task %s with interval %s ms", task.getTaskName(), interval); + } + } diff --git a/model/storage-private/src/main/java/org/keycloak/storage/managers/UserStorageSyncManager.java b/model/storage-private/src/main/java/org/keycloak/storage/managers/UserStorageSyncManager.java index 0086ee2de8..e9d46bd8b9 100755 --- a/model/storage-private/src/main/java/org/keycloak/storage/managers/UserStorageSyncManager.java +++ b/model/storage-private/src/main/java/org/keycloak/storage/managers/UserStorageSyncManager.java @@ -35,6 +35,7 @@ import org.keycloak.storage.UserStorageProviderModel; import org.keycloak.storage.user.ImportSynchronization; import org.keycloak.storage.user.SynchronizationResult; import org.keycloak.timer.TimerProvider; +import org.keycloak.timer.TimerProvider.TimerTaskContext; import java.util.Objects; import java.util.concurrent.Callable; @@ -66,7 +67,7 @@ public class UserStorageSyncManager { providers.forEachOrdered(provider -> { UserStorageProviderFactory factory = (UserStorageProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(UserStorageProvider.class, provider.getProviderId()); if (factory instanceof ImportSynchronization && provider.isImportEnabled()) { - refreshPeriodicSyncForProvider(sessionFactory, timer, provider, realm.getId()); + refreshPeriodicSyncForProvider(sessionFactory, timer, provider, realm); } }); }); @@ -198,58 +199,83 @@ public class UserStorageSyncManager { // Executed once it receives notification that some UserFederationProvider was created or updated - protected static void refreshPeriodicSyncForProvider(final KeycloakSessionFactory sessionFactory, TimerProvider timer, final UserStorageProviderModel provider, final String realmId) { - logger.debugf("Going to refresh periodic sync for provider '%s' . Full sync period: %d , changed users sync period: %d", - provider.getName(), provider.getFullSyncPeriod(), provider.getChangedSyncPeriod()); + protected static void refreshPeriodicSyncForProvider(final KeycloakSessionFactory sessionFactory, TimerProvider timer, final UserStorageProviderModel provider, final RealmModel realm) { + logger.debugf("Going to refresh periodic sync settings for provider '%s' in realm '%s' with realmId '%s'. Full sync period: %d , changed users sync period: %d", + provider.getName(), realm.getName(), realm.getId(), provider.getFullSyncPeriod(), provider.getChangedSyncPeriod()); + String fullSyncTaskName = createSyncTaskName(provider, UserStorageSyncTask.SyncMode.FULL); if (provider.getFullSyncPeriod() > 0) { - // We want periodic full sync for this provider - timer.schedule(new Runnable() { - - @Override - public void run() { - try { - boolean shouldPerformSync = shouldPerformNewPeriodicSync(provider.getLastSync(), provider.getChangedSyncPeriod()); - if (shouldPerformSync) { - syncAllUsers(sessionFactory, realmId, provider); - } else { - logger.debugf("Ignored periodic full sync with storage provider %s due small time since last sync", provider.getName()); - } - } catch (Throwable t) { - logger.error("Error occurred during full sync of users", t); - } - } - - }, provider.getFullSyncPeriod() * 1000, provider.getId() + "-FULL"); + // schedule periodic full sync for this provider + UserStorageSyncTask task = new UserStorageSyncTask(provider, realm, sessionFactory, UserStorageSyncTask.SyncMode.FULL); + timer.schedule(task, provider.getFullSyncPeriod() * 1000, fullSyncTaskName); } else { - timer.cancelTask(provider.getId() + "-FULL"); + // cancel potentially dangling task + timer.cancelTask(fullSyncTaskName); } + String changedSyncTaskName = createSyncTaskName(provider, UserStorageSyncTask.SyncMode.CHANGED); if (provider.getChangedSyncPeriod() > 0) { - // We want periodic sync of just changed users for this provider - timer.schedule(new Runnable() { + // schedule periodic changed user sync for this provider + UserStorageSyncTask task = new UserStorageSyncTask(provider, realm, sessionFactory, UserStorageSyncTask.SyncMode.CHANGED); + timer.schedule(task, provider.getChangedSyncPeriod() * 1000, changedSyncTaskName); + } else { + // cancel potentially dangling task + timer.cancelTask(changedSyncTaskName); + } + } - @Override - public void run() { - try { - boolean shouldPerformSync = shouldPerformNewPeriodicSync(provider.getLastSync(), provider.getChangedSyncPeriod()); - if (shouldPerformSync) { - syncChangedUsers(sessionFactory, realmId, provider); - } else { - logger.debugf("Ignored periodic changed-users sync with storage provider %s due small time since last sync", provider.getName()); - } - } catch (Throwable t) { - logger.error("Error occurred during sync of changed users", t); - } + public static class UserStorageSyncTask implements Runnable { + + private final UserStorageProviderModel provider; + + private final RealmModel realm; + + private final KeycloakSessionFactory sessionFactory; + + private final SyncMode syncMode; + + public static enum SyncMode { + FULL, CHANGED + } + + public UserStorageSyncTask(UserStorageProviderModel provider, RealmModel realm, KeycloakSessionFactory sessionFactory, SyncMode syncMode) { + this.provider = provider; + this.realm = realm; + this.sessionFactory = sessionFactory; + this.syncMode = syncMode; + } + + @Override + public void run() { + + try { + boolean shouldPerformSync = shouldPerformNewPeriodicSync(provider.getLastSync(), provider.getChangedSyncPeriod()); + + if (!shouldPerformSync) { + logger.debugf("Ignored periodic %s users-sync with storage provider %s due small time since last sync in realm %s", // + syncMode, provider.getName(), realm.getName()); + return; } - }, provider.getChangedSyncPeriod() * 1000, provider.getId() + "-CHANGED"); - - } else { - timer.cancelTask(provider.getId() + "-CHANGED"); + switch (syncMode) { + case FULL: + syncAllUsers(sessionFactory, realm.getId(), provider); + break; + case CHANGED: + syncChangedUsers(sessionFactory, realm.getId(), provider); + break; + } + } catch (Throwable t) { + logger.errorf(t,"Error occurred during %s users-sync in realm %s", // + syncMode, realm.getName()); + } } } + public static String createSyncTaskName(UserStorageProviderModel model, UserStorageSyncTask.SyncMode syncMode) { + return UserStorageSyncTask.class.getSimpleName() + "-" + model.getId() + "-" + syncMode; + } + // Skip syncing if there is short time since last sync time. private static boolean shouldPerformNewPeriodicSync(int lastSyncTime, int period) { if (lastSyncTime <= 0) { @@ -264,9 +290,17 @@ public class UserStorageSyncManager { // Executed once it receives notification that some UserFederationProvider was removed protected static void removePeriodicSyncForProvider(TimerProvider timer, UserStorageProviderModel fedProvider) { - logger.debugf("Removing periodic sync for provider %s", fedProvider.getName()); - timer.cancelTask(fedProvider.getId() + "-FULL"); - timer.cancelTask(fedProvider.getId() + "-CHANGED"); + cancelPeriodicSyncForProviderIfPresent(timer, fedProvider, UserStorageSyncTask.SyncMode.FULL); + cancelPeriodicSyncForProviderIfPresent(timer, fedProvider, UserStorageSyncTask.SyncMode.CHANGED); + } + + protected static void cancelPeriodicSyncForProviderIfPresent(TimerProvider timer, UserStorageProviderModel providerModel, UserStorageSyncTask.SyncMode syncMode) { + String taskName = createSyncTaskName(providerModel, syncMode); + TimerTaskContext existingTask = timer.cancelTask(taskName); + if (existingTask != null) { + logger.debugf("Cancelled periodic sync task with task-name '%s' for provider with id '%s' and name '%s'", + taskName, providerModel.getId(), providerModel.getName()); + } } // Update interval of last sync for given UserFederationProviderModel. Do it in separate transaction @@ -310,7 +344,8 @@ public class UserStorageSyncManager { if (fedEvent.isRemoved()) { removePeriodicSyncForProvider(timer, fedEvent.getStorageProvider()); } else { - refreshPeriodicSyncForProvider(sessionFactory, timer, fedEvent.getStorageProvider(), fedEvent.getRealmId()); + RealmModel realm = session.realms().getRealm(fedEvent.getRealmId()); + refreshPeriodicSyncForProvider(sessionFactory, timer, fedEvent.getStorageProvider(), realm); } } diff --git a/server-spi-private/src/main/java/org/keycloak/services/scheduled/ScheduledTaskRunner.java b/server-spi-private/src/main/java/org/keycloak/services/scheduled/ScheduledTaskRunner.java index d4233fb8d3..a4861c8d69 100644 --- a/server-spi-private/src/main/java/org/keycloak/services/scheduled/ScheduledTaskRunner.java +++ b/server-spi-private/src/main/java/org/keycloak/services/scheduled/ScheduledTaskRunner.java @@ -22,25 +22,28 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.timer.ScheduledTask; +import org.keycloak.timer.TaskRunner; /** * @author Stian Thorgersen */ -public class ScheduledTaskRunner implements Runnable { +public class ScheduledTaskRunner implements TaskRunner { private static final Logger logger = Logger.getLogger(ScheduledTaskRunner.class); protected final KeycloakSessionFactory sessionFactory; + protected final ScheduledTask task; - private int transactionLimit; + + protected final int transactionLimit; public ScheduledTaskRunner(KeycloakSessionFactory sessionFactory, ScheduledTask task) { - this.sessionFactory = sessionFactory; - this.task = task; + this(sessionFactory, task, 0); } public ScheduledTaskRunner(KeycloakSessionFactory sessionFactory, ScheduledTask task, int transactionLimit) { - this(sessionFactory, task); + this.sessionFactory = sessionFactory; + this.task = task; this.transactionLimit = transactionLimit; } @@ -61,14 +64,18 @@ public class ScheduledTaskRunner implements Runnable { } }); } catch (Throwable t) { - logger.errorf(t, "Failed to run scheduled task %s", task.getClass().getSimpleName()); + logger.errorf(t, "Failed to run scheduled task %s", task.getTaskName()); } } protected void runTask(KeycloakSession session) { task.run(session); - logger.debug("Executed scheduled task " + task.getClass().getSimpleName()); + logger.debugf("Executed scheduled task %s", task.getTaskName()); } + @Override + public ScheduledTask getTask() { + return task; + } } diff --git a/server-spi-private/src/main/java/org/keycloak/timer/ScheduledTask.java b/server-spi-private/src/main/java/org/keycloak/timer/ScheduledTask.java index 8fe4721b0a..d584ff74ce 100644 --- a/server-spi-private/src/main/java/org/keycloak/timer/ScheduledTask.java +++ b/server-spi-private/src/main/java/org/keycloak/timer/ScheduledTask.java @@ -17,13 +17,15 @@ package org.keycloak.timer; -import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionTask; /** * @author Stian Thorgersen */ -public interface ScheduledTask { +public interface ScheduledTask extends KeycloakSessionTask { - void run(KeycloakSession session); + default String getTaskName() { + return getClass().getSimpleName(); + } } diff --git a/server-spi-private/src/main/java/org/keycloak/timer/TaskRunner.java b/server-spi-private/src/main/java/org/keycloak/timer/TaskRunner.java new file mode 100644 index 0000000000..f559371c48 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/timer/TaskRunner.java @@ -0,0 +1,37 @@ +/* + * Copyright 2023 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.timer; + +/** + * Wrapper around {@link ScheduledTask}. + */ +public interface TaskRunner extends Runnable { + + /** + * Returns the task. + * @return + */ + ScheduledTask getTask(); + + /** + * Name of the task. + * @return + */ + default String getTaskName() { + return getTask().getTaskName(); + } +} diff --git a/server-spi-private/src/main/java/org/keycloak/timer/TimerProvider.java b/server-spi-private/src/main/java/org/keycloak/timer/TimerProvider.java index 7b27941dbf..d654b418ad 100644 --- a/server-spi-private/src/main/java/org/keycloak/timer/TimerProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/timer/TimerProvider.java @@ -26,8 +26,15 @@ public interface TimerProvider extends Provider { public void schedule(Runnable runnable, long intervalMillis, String taskName); + default void schedule(TaskRunner runner, long intervalMillis) { + schedule(runner, intervalMillis, runner.getTaskName()); + } + public void scheduleTask(ScheduledTask scheduledTask, long intervalMillis, String taskName); + public default void scheduleTask(ScheduledTask scheduledTask, long intervalMillis) { + scheduleTask(scheduledTask, intervalMillis, scheduledTask.getTaskName()); + } /** * Cancel task and return the details about it, so it can be eventually restored later diff --git a/services/src/main/java/org/keycloak/services/managers/RealmManager.java b/services/src/main/java/org/keycloak/services/managers/RealmManager.java index 1881e70f67..1d3eab066e 100755 --- a/services/src/main/java/org/keycloak/services/managers/RealmManager.java +++ b/services/src/main/java/org/keycloak/services/managers/RealmManager.java @@ -19,6 +19,7 @@ package org.keycloak.services.managers; import org.keycloak.Config; import org.keycloak.common.Profile; import org.keycloak.common.enums.SslRequired; +import org.keycloak.models.AbstractKeycloakTransaction; import org.keycloak.models.AccountRoles; import org.keycloak.models.AdminRoles; import org.keycloak.models.BrowserSecurityHeaders; @@ -606,8 +607,19 @@ public class RealmManager { KeycloakModelUtils.setupDeleteAccount(realm.getClientByClientId(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID)); } - // Refresh periodic sync tasks for configured storageProviders - StoreSyncEvent.fire(session, realm, false); + // enlistAfterCompletion(..) as we need to ensure that the realm is committed to the database before we can update the sync tasks + session.getTransactionManager().enlistAfterCompletion(new AbstractKeycloakTransaction() { + @Override + protected void commitImpl() { + // Refresh periodic sync tasks for configured storageProviders + StoreSyncEvent.fire(session, realm, false); + } + + @Override + protected void rollbackImpl() { + // NOOP + } + }); setupAuthorizationServices(realm); setupClientRegistrations(realm); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java index 9eaa2d9248..8394570adf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java @@ -91,16 +91,15 @@ public class SyncFederationTest extends AbstractAuthTest { DummyUserFederationProviderFactory dummyFedFactory = (DummyUserFederationProviderFactory) sessionFactory.getProviderFactory(UserStorageProvider.class, DummyUserFederationProviderFactory.PROVIDER_NAME); // Assert that after some period was DummyUserFederationProvider triggered - UserStorageSyncManager usersSyncManager = new UserStorageSyncManager(); sleep(1800); // Cancel timer - usersSyncManager.notifyToRefreshPeriodicSync(session, appRealm, dummyModel, true); + UserStorageSyncManager.notifyToRefreshPeriodicSync(session, appRealm, dummyModel, true); log.infof("Notified sync manager about cancel periodic sync"); // This sync is here just to ensure that we have lock (doublecheck that periodic sync, which was possibly triggered before canceling timer is finished too) while (true) { - SynchronizationResult result = usersSyncManager.syncChangedUsers(session.getKeycloakSessionFactory(), appRealm.getId(), dummyModel); + SynchronizationResult result = UserStorageSyncManager.syncChangedUsers(session.getKeycloakSessionFactory(), appRealm.getId(), dummyModel); if (result.isIgnored()) { log.infof("Still waiting for lock before periodic sync is finished", result.toString()); sleep(1000);