diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java index 8442ec3fb..713039fb3 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java @@ -8,6 +8,7 @@ */ package org.eclipse.hawkbit.repository.jpa; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Map; @@ -109,6 +110,9 @@ public class JpaRolloutManagement extends AbstractRolloutManagement { */ private static final int TRANSACTION_ACTIONS = 5_000; + private static final List ACTIVE_ROLLOUTS = Arrays.asList(RolloutStatus.CREATING, + RolloutStatus.DELETING, RolloutStatus.STARTING, RolloutStatus.READY, RolloutStatus.RUNNING); + @Autowired private RolloutRepository rolloutRepository; @@ -716,31 +720,30 @@ public class JpaRolloutManagement extends AbstractRolloutManagement { // No transaction, will be created per handled rollout @Transactional(propagation = Propagation.NEVER) public void handleRollouts() { - rolloutRepository - .findByStatusIn(Lists.newArrayList(RolloutStatus.CREATING, RolloutStatus.DELETING, - RolloutStatus.STARTING, RolloutStatus.READY, RolloutStatus.RUNNING)) - .forEach(this::handleRollout); - } + final List rollouts = rolloutRepository.findByStatusIn(ACTIVE_ROLLOUTS); - private void handleRollout(final Long rolloutId) { - LOGGER.debug("handleRollout called for rollout {}", rolloutId); + if (rollouts.isEmpty()) { + return; + } final String tenant = tenantAware.getCurrentTenant(); - final String handlerId = tenant + "-rollout-" + rolloutId; + final String handlerId = tenant + "-rollout"; final Lock lock = lockRegistry.obtain(handlerId); if (!lock.tryLock()) { return; } try { - runInNewTransaction(handlerId, status -> executeFittingHandler(rolloutId)); + rollouts.forEach(rolloutId -> runInNewTransaction(handlerId + "-" + rolloutId, + status -> executeFittingHandler(rolloutId))); } finally { lock.unlock(); } } private int executeFittingHandler(final Long rolloutId) { + LOGGER.debug("handle rollout {}", rolloutId); final JpaRollout rollout = rolloutRepository.findOne(rolloutId); switch (rollout.getStatus()) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java index 188d9db51..a75c3bc1f 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java @@ -9,7 +9,6 @@ package org.eclipse.hawkbit.repository.jpa; import java.util.Map; -import java.util.concurrent.Executor; import javax.persistence.EntityManager; import javax.sql.DataSource; @@ -70,7 +69,6 @@ import org.eclipse.hawkbit.tenancy.TenantAware; import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.orm.jpa.JpaBaseConfiguration; @@ -545,7 +543,7 @@ public class RepositoryApplicationConfiguration extends JpaBaseConfiguration { * @param autoAssignChecker * to run a check as tenant * @param lockRegistry - * to lock the tenant for auto assigment + * to lock the tenant for auto assignment * @return a new {@link AutoAssignChecker} */ @Bean @@ -575,8 +573,6 @@ public class RepositoryApplicationConfiguration extends JpaBaseConfiguration { * to run the rollout handler * @param systemSecurityContext * to run as system - * @param threadPoolExecutor - * to execute the handlers in parallel * @return a new {@link RolloutScheduler} bean. */ @Bean @@ -584,9 +580,7 @@ public class RepositoryApplicationConfiguration extends JpaBaseConfiguration { @Profile("!test") @ConditionalOnProperty(prefix = "hawkbit.rollout.scheduler", name = "enabled", matchIfMissing = true) RolloutScheduler rolloutScheduler(final TenantAware tenantAware, final SystemManagement systemManagement, - final RolloutManagement rolloutManagement, final SystemSecurityContext systemSecurityContext, - @Qualifier("asyncExecutor") final Executor threadPoolExecutor) { - return new RolloutScheduler(tenantAware, systemManagement, rolloutManagement, systemSecurityContext, - threadPoolExecutor); + final RolloutManagement rolloutManagement, final SystemSecurityContext systemSecurityContext) { + return new RolloutScheduler(tenantAware, systemManagement, rolloutManagement, systemSecurityContext); } } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignScheduler.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignScheduler.java index 174a47359..0bbb2516a 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignScheduler.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/autoassign/AutoAssignScheduler.java @@ -84,20 +84,21 @@ public class AutoAssignScheduler { // each tenant separately. final List tenants = systemManagement.findTenants(); LOGGER.info("Checking target filter queries for tenants: {}", tenants.size()); - for (final String tenant : tenants) { - final Lock lock = lockRegistry.obtain(tenant + "-autoassign"); - if (!lock.tryLock()) { - return null; - } - try { + final Lock lock = lockRegistry.obtain("autoassign"); + if (!lock.tryLock()) { + return null; + } + + try { + for (final String tenant : tenants) { tenantAware.runAsTenant(tenant, () -> { autoAssignChecker.check(); return null; }); - } finally { - lock.unlock(); } + } finally { + lock.unlock(); } return null; } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/RolloutScheduler.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/RolloutScheduler.java index 37c9f86b1..c27bd2077 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/RolloutScheduler.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rollout/RolloutScheduler.java @@ -9,9 +9,6 @@ package org.eclipse.hawkbit.repository.jpa.rollout; import java.util.List; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Executor; -import java.util.concurrent.ExecutorCompletionService; import org.eclipse.hawkbit.repository.RolloutManagement; import org.eclipse.hawkbit.repository.SystemManagement; @@ -21,8 +18,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.scheduling.annotation.Scheduled; -import com.google.common.base.Throwables; - /** * Scheduler to schedule the {@link RolloutManagement#handleRollouts()}. The * delay between the checks be be configured using the property from @@ -42,8 +37,6 @@ public class RolloutScheduler { private final SystemSecurityContext systemSecurityContext; - private final ExecutorCompletionService completionService; - /** * Constructor. * @@ -55,17 +48,13 @@ public class RolloutScheduler { * to run the rollout handler * @param systemSecurityContext * to run as system - * @param threadPoolExecutor - * to execute the handlers in parallel */ public RolloutScheduler(final TenantAware tenantAware, final SystemManagement systemManagement, - final RolloutManagement rolloutManagement, final SystemSecurityContext systemSecurityContext, - final Executor threadPoolExecutor) { + final RolloutManagement rolloutManagement, final SystemSecurityContext systemSecurityContext) { this.tenantAware = tenantAware; this.systemManagement = systemManagement; this.rolloutManagement = rolloutManagement; this.systemSecurityContext = systemSecurityContext; - completionService = new ExecutorCompletionService<>(threadPoolExecutor); } /** @@ -80,7 +69,7 @@ public class RolloutScheduler { // run this code in system code privileged to have the necessary // permission to query and create entities. - final int tasks = systemSecurityContext.runAsSystem(() -> { + systemSecurityContext.runAsSystem(() -> { // workaround eclipselink that is currently not possible to // execute a query without multitenancy if MultiTenant // annotation is used. @@ -90,25 +79,21 @@ public class RolloutScheduler { final List tenants = systemManagement.findTenants(); LOGGER.info("Checking rollouts for {} tenants", tenants.size()); for (final String tenant : tenants) { - completionService.submit(() -> tenantAware.runAsTenant(tenant, () -> { - rolloutManagement.handleRollouts(); + tenantAware.runAsTenant(tenant, () -> { + try { + rolloutManagement.handleRollouts(); + // We catch all potential runtime exceptions here to + // ensure that not all tenants are blocked if we have a + // problem with a rollout. + } catch (@SuppressWarnings("squid:S1166") final RuntimeException e) { + LOGGER.error("Failed to handle rollouts for tenant {}. I will move on to next tenant.", tenant, + e); + } return null; - })); + }); } - return tenants.size(); + return null; }); - - waitUntilHandlersAreComplete(tasks); } - private void waitUntilHandlersAreComplete(final int tasks) { - try { - for (int i = 0; i < tasks; i++) { - completionService.take().get(); - } - } catch (InterruptedException | ExecutionException e) { - Thread.currentThread().interrupt(); - throw Throwables.propagate(e); - } - } }