diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java index bde30a1ef6..22f1423cb7 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java @@ -20,6 +20,7 @@ public enum FeatureFlagEnum { // Deprecated CE flags over here release_git_autocommit_feature_enabled, release_git_autocommit_eligibility_enabled, + release_dynamodb_connection_time_to_live_enabled, // Add EE flags below this line, to avoid conflicts. } diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java index 5f9cdb89d1..3613857392 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java @@ -295,6 +295,39 @@ public interface PluginExecutor extends ExtensionPoint, CrudTemplateService { Map featureFlagMap) { return this.trigger(connection, datasourceConfiguration, request); } + /* + * Feature flagging implementation is added here as a potential fix to dynamoDB query timeouts problem + * This is a temporary fix and will be removed once we get the confirmation from the user that issue is resolved + * Even if the issue is not resolved, we will know that fix does not work and hence will be removing the code in any case + * https://github.com/appsmithorg/appsmith/issues/39426 Created task here to remove this flag + * This implementation ensures that none of the existing plugins have any impact due to feature flagging, hence if else condition + * This applies to both datasourceCreate and testDatasource methods added below + * */ + default Mono datasourceCreate( + DatasourceConfiguration datasourceConfiguration, Boolean isDynamoDBConnectionTimeToLiveEnabled) { + return this.datasourceCreate(datasourceConfiguration); + } + + default Mono testDatasource( + DatasourceConfiguration datasourceConfiguration, Boolean isDynamoDBConnectionTimeToLiveEnabled) { + return this.datasourceCreate(datasourceConfiguration, isDynamoDBConnectionTimeToLiveEnabled) + .flatMap(connection -> { + return this.testDatasource(connection).doFinally(signal -> this.datasourceDestroy(connection)); + }) + .onErrorResume(error -> { + // We always expect to have an error object, but the error object may not be well-formed + final String errorMessage = error.getMessage() == null + ? AppsmithPluginError.PLUGIN_DATASOURCE_TEST_GENERIC_ERROR.getMessage() + : error.getMessage(); + if (error instanceof AppsmithPluginException + && StringUtils.hasLength(((AppsmithPluginException) error).getDownstreamErrorMessage())) { + return Mono.just(new DatasourceTestResult( + ((AppsmithPluginException) error).getDownstreamErrorMessage(), errorMessage)); + } + return Mono.just(new DatasourceTestResult(errorMessage)); + }) + .subscribeOn(Schedulers.boundedElastic()); + } /** * This function is responsible for preparing the action and datasource configurations to be ready for execution. diff --git a/app/server/appsmith-plugins/dynamoPlugin/pom.xml b/app/server/appsmith-plugins/dynamoPlugin/pom.xml index 54e544bc7a..d1febad2ed 100644 --- a/app/server/appsmith-plugins/dynamoPlugin/pom.xml +++ b/app/server/appsmith-plugins/dynamoPlugin/pom.xml @@ -32,6 +32,23 @@ + + + software.amazon.awssdk + apache-client + 2.15.3 + compile + + + org.slf4j + slf4j-api + + + com.fasterxml.jackson.core + jackson-databind + + + diff --git a/app/server/appsmith-plugins/dynamoPlugin/src/main/java/com/external/plugins/DynamoPlugin.java b/app/server/appsmith-plugins/dynamoPlugin/src/main/java/com/external/plugins/DynamoPlugin.java index 01949363b7..39b5f9e09a 100644 --- a/app/server/appsmith-plugins/dynamoPlugin/src/main/java/com/external/plugins/DynamoPlugin.java +++ b/app/server/appsmith-plugins/dynamoPlugin/src/main/java/com/external/plugins/DynamoPlugin.java @@ -29,6 +29,7 @@ import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; import software.amazon.awssdk.core.SdkBytes; import software.amazon.awssdk.core.SdkField; import software.amazon.awssdk.core.SdkPojo; +import software.amazon.awssdk.http.apache.ApacheHttpClient; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; import software.amazon.awssdk.services.dynamodb.DynamoDbClientBuilder; @@ -42,6 +43,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.lang.reflect.WildcardType; import java.net.URI; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -70,6 +72,7 @@ public class DynamoPlugin extends BasePlugin { private static final String DYNAMO_TYPE_BINARY_SET_LABEL = "BS"; private static final String DYNAMO_TYPE_MAP_LABEL = "M"; private static final String DYNAMO_TYPE_LIST_LABEL = "L"; + private static final Duration CONNECTION_TIME_TO_LIVE = Duration.ofDays(1); public DynamoPlugin(PluginWrapper wrapper) { super(wrapper); @@ -280,6 +283,40 @@ public class DynamoPlugin extends BasePlugin { .subscribeOn(scheduler); } + @Override + public Mono datasourceCreate( + DatasourceConfiguration datasourceConfiguration, Boolean isFlagEnabled) { + log.debug(Thread.currentThread().getName() + ": datasourceCreate() called for Dynamo plugin."); + return Mono.fromCallable(() -> { + log.debug(Thread.currentThread().getName() + ": creating dynamodbclient from DynamoDB plugin."); + + /** + * Current understanding is that the issue mentioned in #6030 is because of is because of connection object getting stale. + * Setting connection to live value to 1 day is a precaution move to make sure that we don't land into a situation + * where an older connection can malfunction. + * To understand what this config means please check here: + * {@link https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#connectionTimeToLive(java.time.Duration)} + */ + DynamoDbClientBuilder builder; + if (isFlagEnabled) { + log.debug("DynamoDB client builder created with custom connection time to live"); + builder = DynamoDbClient.builder() + .httpClient(ApacheHttpClient.builder() + .connectionTimeToLive(CONNECTION_TIME_TO_LIVE) + .build()); + } else { + builder = DynamoDbClient.builder(); + } + + return getDynamoDBClientObject(datasourceConfiguration, builder); + }) + .subscribeOn(scheduler); + } + + // This method is not being used right now as we had created a separate method with the same name in the + // DynamoPlugin class. + // which creates dynamoDB client based on feature flagging + // Once feature flagging is removed, we can go back to using this method. @Override public Mono datasourceCreate(DatasourceConfiguration datasourceConfiguration) { log.debug(Thread.currentThread().getName() + ": datasourceCreate() called for Dynamo plugin."); @@ -287,26 +324,7 @@ public class DynamoPlugin extends BasePlugin { log.debug(Thread.currentThread().getName() + ": creating dynamodbclient from DynamoDB plugin."); final DynamoDbClientBuilder builder = DynamoDbClient.builder(); - if (!CollectionUtils.isEmpty(datasourceConfiguration.getEndpoints())) { - final Endpoint endpoint = - datasourceConfiguration.getEndpoints().get(0); - builder.endpointOverride( - URI.create("http://" + endpoint.getHost() + ":" + endpoint.getPort())); - } - - final DBAuth authentication = (DBAuth) datasourceConfiguration.getAuthentication(); - if (authentication == null || !StringUtils.hasLength(authentication.getDatabaseName())) { - throw new AppsmithPluginException( - AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, - DynamoErrorMessages.MISSING_REGION_ERROR_MSG); - } - - builder.region(Region.of(authentication.getDatabaseName())); - - builder.credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create( - authentication.getUsername(), authentication.getPassword()))); - - return builder.build(); + return getDynamoDBClientObject(datasourceConfiguration, builder); }) .subscribeOn(scheduler); } @@ -611,4 +629,25 @@ public class DynamoPlugin extends BasePlugin { } return true; } + + private static DynamoDbClient getDynamoDBClientObject( + DatasourceConfiguration dsConfig, DynamoDbClientBuilder builder) { + if (!CollectionUtils.isEmpty(dsConfig.getEndpoints())) { + final Endpoint endpoint = dsConfig.getEndpoints().get(0); + builder.endpointOverride(URI.create("http://" + endpoint.getHost() + ":" + endpoint.getPort())); + } + + final DBAuth authentication = (DBAuth) dsConfig.getAuthentication(); + if (authentication == null || !StringUtils.hasLength(authentication.getDatabaseName())) { + throw new AppsmithPluginException( + AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, DynamoErrorMessages.MISSING_REGION_ERROR_MSG); + } + + builder.region(Region.of(authentication.getDatabaseName())); + + builder.credentialsProvider(StaticCredentialsProvider.create( + AwsBasicCredentials.create(authentication.getUsername(), authentication.getPassword()))); + + return builder.build(); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java index df7decf159..591ea75890 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java @@ -661,8 +661,24 @@ public class DatasourceServiceCEImpl implements DatasourceServiceCE { .switchIfEmpty(Mono.error(new AppsmithException( AppsmithError.NO_RESOURCE_FOUND, FieldName.PLUGIN, datasourceStorage.getPluginId()))); - return pluginExecutorMono.flatMap(pluginExecutor -> ((PluginExecutor) pluginExecutor) - .testDatasource(datasourceStorage.getDatasourceConfiguration())); + // Feature flagging implementation is added here as a potential fix to dynamoDB query timeouts problem + // This is a temporary fix and will be removed once we get the confirmation from the user that issue is resolved + // Even if the issue is not resolved, we will know that fix does not work and hence will be removing the code in + // any case + // https://github.com/appsmithorg/appsmith/issues/39426 Created task here to remove this flag + // This implementation ensures that none of the existing plugins have any impact due to feature flagging, hence + // if else condition + return featureFlagService + .check(FeatureFlagEnum.release_dynamodb_connection_time_to_live_enabled) + .flatMap(isDynamoDBConnectionTimeToLiveEnabled -> { + if (isDynamoDBConnectionTimeToLiveEnabled) { + return pluginExecutorMono.flatMap(pluginExecutor -> ((PluginExecutor) pluginExecutor) + .testDatasource(datasourceStorage.getDatasourceConfiguration(), true)); + } else { + return pluginExecutorMono.flatMap(pluginExecutor -> ((PluginExecutor) pluginExecutor) + .testDatasource(datasourceStorage.getDatasourceConfiguration())); + } + }); } /* diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/DatasourceContextServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/DatasourceContextServiceImpl.java index 1a686d1635..c2d77e79a2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/DatasourceContextServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/DatasourceContextServiceImpl.java @@ -20,7 +20,8 @@ public class DatasourceContextServiceImpl extends DatasourceContextServiceCEImpl PluginService pluginService, PluginExecutorHelper pluginExecutorHelper, ConfigService configService, - DatasourcePermission datasourcePermission) { + DatasourcePermission datasourcePermission, + FeatureFlagService featureFlagService) { super( datasourceService, @@ -28,6 +29,7 @@ public class DatasourceContextServiceImpl extends DatasourceContextServiceCEImpl pluginService, pluginExecutorHelper, configService, - datasourcePermission); + datasourcePermission, + featureFlagService); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java index 69632105e0..d4a9682101 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java @@ -3,6 +3,7 @@ package com.appsmith.server.services.ce; import com.appsmith.external.constants.PluginConstants; import com.appsmith.external.dtos.ExecutePluginDTO; import com.appsmith.external.dtos.RemoteDatasourceDTO; +import com.appsmith.external.enums.FeatureFlagEnum; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.exceptions.pluginExceptions.StaleConnectionException; import com.appsmith.external.models.DatasourceStorage; @@ -20,6 +21,7 @@ import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.PluginExecutorHelper; import com.appsmith.server.plugins.base.PluginService; import com.appsmith.server.services.ConfigService; +import com.appsmith.server.services.FeatureFlagService; import com.appsmith.server.solutions.DatasourcePermission; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; @@ -68,6 +70,7 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC private final PluginExecutorHelper pluginExecutorHelper; private final ConfigService configService; private final DatasourcePermission datasourcePermission; + private final FeatureFlagService featureFlagService; private final AppsmithException TOO_MANY_REQUESTS_EXCEPTION = new AppsmithException(AppsmithError.TOO_MANY_FAILED_DATASOURCE_CONNECTION_REQUESTS); @@ -79,7 +82,8 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC PluginService pluginService, PluginExecutorHelper pluginExecutorHelper, ConfigService configService, - DatasourcePermission datasourcePermission) { + DatasourcePermission datasourcePermission, + FeatureFlagService featureFlagService) { this.datasourceService = datasourceService; this.datasourceStorageService = datasourceStorageService; this.pluginService = pluginService; @@ -89,6 +93,7 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC this.datasourceContextSynchronizationMonitorMap = new ConcurrentHashMap<>(); this.configService = configService; this.datasourcePermission = datasourcePermission; + this.featureFlagService = featureFlagService; } private RemovalListener createRemovalListener() { @@ -209,8 +214,27 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC /* Create a fresh datasource context */ DatasourceContext datasourceContext = new DatasourceContext<>(); - Mono connectionMonoCache = pluginExecutor - .datasourceCreate(datasourceStorage.getDatasourceConfiguration()) + + // Feature flagging implementation is added here as a potential fix to dynamoDB query timeouts + // problem + // This is a temporary fix and will be removed once we get the confirmation from the user that + // issue is resolved + // Even if the issue is not resolved, we will know that fix does not work and hence will be + // removing the code in any case + // https://github.com/appsmithorg/appsmith/issues/39426 Created task here to remove this flag + // This implementation ensures that none of the existing plugins have any impact due to feature + // flagging, hence if else condition + Mono connectionMonoCache = featureFlagService + .check(FeatureFlagEnum.release_dynamodb_connection_time_to_live_enabled) + .flatMap(isDynamoDBConnectionTimeToLiveEnabled -> { + if (isDynamoDBConnectionTimeToLiveEnabled) { + return pluginExecutor.datasourceCreate( + datasourceStorage.getDatasourceConfiguration(), true); + } else { + return pluginExecutor.datasourceCreate( + datasourceStorage.getDatasourceConfiguration()); + } + }) .cache(); Mono> datasourceContextMonoCache = connectionMonoCache