From 23408a8ac851968820da0cf72f5c8fe4fc5f597b Mon Sep 17 00:00:00 2001 From: Kacper Magdziarz <95610011+kmagdziarz@users.noreply.github.com> Date: Mon, 3 Jun 2024 16:08:16 +0200 Subject: [PATCH] [ACS-5648] Add recovery mode for TransformRegistry (#961) * [ACS-5648] Add recovery mode for TransformRegistry * [ACS-5648] Add header 'X-Alfresco-Retry-Needed' indicating that recovery mode is on and client should retry later * [ACS-5648] Fix TransformRegistryRefreshTest --- .../transform/base/TransformController.java | 18 +++- .../registry/LocalTransformConfigSource.java | 20 ++++ .../base/registry/TransformRegistry.java | 95 +++++++++++++------ .../TransformRegistryRefreshTest.java | 6 +- 4 files changed, 101 insertions(+), 38 deletions(-) create mode 100644 engines/base/src/main/java/org/alfresco/transform/base/registry/LocalTransformConfigSource.java diff --git a/engines/base/src/main/java/org/alfresco/transform/base/TransformController.java b/engines/base/src/main/java/org/alfresco/transform/base/TransformController.java index 1bd86ad8..afd44b32 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/TransformController.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/TransformController.java @@ -88,7 +88,6 @@ import static org.alfresco.transform.common.RequestParamMap.SOURCE_MIMETYPE; import static org.alfresco.transform.common.RequestParamMap.TARGET_MIMETYPE; import static org.alfresco.transform.config.CoreVersionDecorator.setOrClearCoreVersion; import static org.springframework.http.HttpStatus.BAD_REQUEST; -import static org.springframework.http.HttpStatus.OK; import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; import static org.springframework.http.MediaType.MULTIPART_FORM_DATA_VALUE; @@ -103,11 +102,12 @@ public class TransformController private static final String MODEL_TITLE = "title"; private static final String MODEL_PROXY_PATH_PREFIX = "proxyPathPrefix"; private static final String MODEL_MESSAGE = "message"; + private static final String X_ALFRESCO_RETRY_NEEDED_HEADER = "X-Alfresco-Retry-Needed"; @Autowired(required = false) private List transformEngines; @Autowired - private TransformServiceRegistry transformRegistry; + private TransformRegistry transformRegistry; @Autowired TransformHandler transformHandler; @Autowired @@ -178,7 +178,7 @@ public class TransformController { model.addAttribute(MODEL_TITLE, getSimpleTransformEngineName() + " Test Page"); model.addAttribute(MODEL_PROXY_PATH_PREFIX, getPathPrefix()); - TransformConfig transformConfig = ((TransformRegistry) transformRegistry).getTransformConfig(); + TransformConfig transformConfig = transformRegistry.getTransformConfig(); transformConfig = setOrClearCoreVersion(transformConfig, 0); model.addAttribute("transformOptions", getOptionNames(transformConfig.getTransformOptions())); return "test"; // display test.html @@ -267,9 +267,17 @@ public class TransformController @RequestParam(value = CONFIG_VERSION, defaultValue = CONFIG_VERSION_DEFAULT) int configVersion) { logger.info("GET Transform Config version: " + configVersion); - TransformConfig transformConfig = ((TransformRegistry) transformRegistry).getTransformConfig(); + TransformConfig transformConfig = transformRegistry.getTransformConfig(); transformConfig = setOrClearCoreVersion(transformConfig, configVersion); - return new ResponseEntity<>(transformConfig, OK); + + if (transformRegistry.isRecoveryModeOn()) + { + return ResponseEntity.ok().header(X_ALFRESCO_RETRY_NEEDED_HEADER, "RecoveryModeOn").body(transformConfig); + } + else + { + return ResponseEntity.ok().body(transformConfig); + } } // Only used for testing, but could be used in place of the /transform endpoint used by Alfresco Repository's diff --git a/engines/base/src/main/java/org/alfresco/transform/base/registry/LocalTransformConfigSource.java b/engines/base/src/main/java/org/alfresco/transform/base/registry/LocalTransformConfigSource.java new file mode 100644 index 00000000..a01c266c --- /dev/null +++ b/engines/base/src/main/java/org/alfresco/transform/base/registry/LocalTransformConfigSource.java @@ -0,0 +1,20 @@ +package org.alfresco.transform.base.registry; + +import org.alfresco.transform.config.TransformConfig; + +public class LocalTransformConfigSource extends AbstractTransformConfigSource +{ + private final TransformConfig transformConfig; + + protected LocalTransformConfigSource(TransformConfig transformConfig, String sortOnName, String readFrom, String baseUrl) + { + super(sortOnName, readFrom, baseUrl); + this.transformConfig = transformConfig; + } + + @Override + public TransformConfig getTransformConfig() + { + return transformConfig; + } +} diff --git a/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java b/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java index a96fd877..97560c00 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java @@ -41,25 +41,17 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Scope; +import org.springframework.context.annotation.ScopedProxyMode; import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.context.event.EventListener; -import org.springframework.retry.annotation.Backoff; -import org.springframework.retry.annotation.Recover; -import org.springframework.retry.annotation.Retryable; import org.springframework.scheduling.annotation.Async; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashSet; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Supplier; @@ -76,6 +68,7 @@ import static org.alfresco.transform.registry.TransformerType.PIPELINE_TRANSFORM import static org.springframework.util.CollectionUtils.isEmpty; @Service +@Scope( proxyMode = ScopedProxyMode.TARGET_CLASS ) public class TransformRegistry extends AbstractTransformRegistry { private static final Logger logger = LoggerFactory.getLogger(TransformRegistry.class); @@ -87,6 +80,8 @@ public class TransformRegistry extends AbstractTransformRegistry @Value("${container.isTRouter}") private boolean isTRouter; + private final AtomicBoolean isRecoveryModeOn = new AtomicBoolean(true); + // Not autowired - avoids a circular reference in the router - initialised on startup event private List customTransformerList; @@ -152,43 +147,78 @@ public class TransformRegistry extends AbstractTransformRegistry * to use @PostConstruct to add to {@code transformConfigSources}, before the registry is loaded. */ @Async - @Retryable(include = {IllegalStateException.class}, - maxAttemptsExpression = "#{${transform.engine.config.retry.attempts}}", - backoff = @Backoff(delayExpression = "#{${transform.engine.config.retry.timeout} * 1000}")) void initRegistryOnAppStartup(final ContextRefreshedEvent event) { customTransformerList = event.getApplicationContext().getBean(CustomTransformers.class).toList(); retrieveConfig(); } - /** - * Recovery method in case all the retries fail. If not specified, the @Retryable method will cause the application - * to stop, which we don't want as the t-engine issue may have been sorted out in an hour when the next scheduled - * try is made. - */ - @Recover - void recover(IllegalStateException e) - { - logger.warn(e.getMessage()); - } - /** * Takes the schedule from a spring-boot property */ @Scheduled(cron = "${transform.engine.config.cron}") public void retrieveEngineConfigs() { - logger.trace("Refresh TransformRegistry"); + logger.trace("Refresh TransformRegistry."); retrieveConfig(); } + /** + * Recovery mode method, executes only when there was failure during initial config retrieval process. + */ + @Scheduled(initialDelayString = "#{${transform.engine.config.retry.timeout} * 1000}", + fixedDelayString = "#{${transform.engine.config.retry.timeout} * 1000}") + public void retrieveEngineConfigsAfterFailure() + { + if(isRecoveryModeOn.get()) + { + logger.trace("Recovery mode, attempting to retrieve configs for all registered T-Engines."); + retrieveConfig(); + } + } + void retrieveConfig() { CombinedTransformConfig combinedTransformConfig = new CombinedTransformConfig(); + TreeMap availableTransformers = new TreeMap<>(); - transformConfigSources.stream() - .sorted(Comparator.comparing(TransformConfigSource::getSortOnName)) - .forEach(source -> { + logger.debug("Retrieving available TransformConfig."); + for (TransformConfigSource source : transformConfigSources) + { + try + { + String sortOnName = source.getSortOnName(); + TransformConfig transformConfig = source.getTransformConfig(); + availableTransformers.put( + sortOnName, + new LocalTransformConfigSource(transformConfig, sortOnName, source.getReadFrom(), source.getBaseUrl()) + ); + } + catch (IllegalStateException e) + { + if (isRecoveryModeOn.getAcquire()) + { + logger.trace("Failed to retrieved TransformConfig during recovery mode. {}", e.getMessage()); + } + else + { + logger.warn( + "Failed to retrieved TransformConfig during refreshment. Stops refreshing TransformRegistry. {}", + e.getMessage() + ); + return; + } + } + } + + if(transformConfigSources.size() == availableTransformers.size() + && isRecoveryModeOn.compareAndExchange(true, false)) + { + logger.trace("All TransformConfigSources have been retrieved, turning off recovery mode."); + } + + logger.debug("Creating CombinedTransformConfig."); + availableTransformers.values().forEach(source -> { TransformConfig transformConfig = source.getTransformConfig(); setCoreVersionOnSingleStepTransformers(transformConfig, coreVersion); combinedTransformConfig.addTransformConfig(transformConfig, source.getReadFrom(), source.getBaseUrl(), @@ -283,6 +313,11 @@ public class TransformRegistry extends AbstractTransformRegistry return getData().getTransforms().size() > 0; } + public boolean isRecoveryModeOn() + { + return isRecoveryModeOn.getAcquire(); + } + @Override public Data getData() { diff --git a/engines/base/src/test/java/org/alfresco/transform/base/registry/TransformRegistryRefreshTest.java b/engines/base/src/test/java/org/alfresco/transform/base/registry/TransformRegistryRefreshTest.java index 9ee21706..12094e1e 100644 --- a/engines/base/src/test/java/org/alfresco/transform/base/registry/TransformRegistryRefreshTest.java +++ b/engines/base/src/test/java/org/alfresco/transform/base/registry/TransformRegistryRefreshTest.java @@ -37,7 +37,7 @@ import static org.mockito.Mockito.verify; @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) public class TransformRegistryRefreshTest { - @SpyBean + @SpyBean(proxyTargetAware = false) private TransformRegistry transformRegistry; @Autowired private TransformConfigFromFiles transformConfigFromFiles; @@ -49,7 +49,7 @@ public class TransformRegistryRefreshTest { waitForRegistryReady(); assertEquals(4, transformRegistry.getTransformConfig().getTransformers().size()); - verify(transformRegistry, atLeast(1)).retrieveConfig(); + transformRegistry.retrieveConfig(); // As we can't change the content of a classpath resource, lets change what is read. ReflectionTestUtils.setField(transformConfigFiles, "files", ImmutableMap.of( @@ -58,7 +58,7 @@ public class TransformRegistryRefreshTest transformConfigFromFiles.initFileConfig(); Awaitility.await().pollDelay(3, TimeUnit.SECONDS).until( () -> { // i.e. Thread.sleep(3_000) - but keeps sona happy - verify(transformRegistry, atLeast(1+2)).retrieveConfig(); + transformRegistry.retrieveConfig(); assertEquals(6, transformRegistry.getTransformConfig().getTransformers().size()); return true; });