From a990515acc76d0ec24fc5b6c66d82447b9e16f4a Mon Sep 17 00:00:00 2001 From: Stanislaw Bieniecki <stanislaw.bieniecki@hitachivantara.com> Date: Thu, 16 Jan 2025 12:59:29 +0100 Subject: [PATCH] Use partition level flag if service level off --- .../src/main/resources/application.properties | 2 +- .../SchemaConverterPropertiesConfig.java | 23 +++++++++++++++++-- .../service/StorageIndexerPayloadMapper.java | 23 +++++++++++++++++-- .../converter/PropertiesProcessorTest.java | 12 ++++++---- .../SchemaToStorageFormatImplTest.java | 6 ++++- .../StorageIndexerPayloadMapperTest.java | 6 ++++- 6 files changed, 61 insertions(+), 11 deletions(-) diff --git a/indexer-core-plus/src/main/resources/application.properties b/indexer-core-plus/src/main/resources/application.properties index 5d827e4f1..47fdb129c 100644 --- a/indexer-core-plus/src/main/resources/application.properties +++ b/indexer-core-plus/src/main/resources/application.properties @@ -63,7 +63,7 @@ rabbitmq-retry-limit=5 # Feature flag settings featureFlag.strategy=dataPartition -featureFlag.mapBooleanToString.enabled=true +featureFlag.mapBooleanToString.enabled=false featureFlag.asIngestedCoordinates.enabled=false featureFlag.keywordLower.enabled=false featureFlag.bagOfWords.enabled=false diff --git a/indexer-core/src/main/java/org/opengroup/osdu/indexer/schema/converter/config/SchemaConverterPropertiesConfig.java b/indexer-core/src/main/java/org/opengroup/osdu/indexer/schema/converter/config/SchemaConverterPropertiesConfig.java index 077ae9925..7ee550a3c 100644 --- a/indexer-core/src/main/java/org/opengroup/osdu/indexer/schema/converter/config/SchemaConverterPropertiesConfig.java +++ b/indexer-core/src/main/java/org/opengroup/osdu/indexer/schema/converter/config/SchemaConverterPropertiesConfig.java @@ -11,6 +11,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.context.properties.ConfigurationProperties; import org.opengroup.osdu.core.common.feature.IFeatureFlag; +import org.opengroup.osdu.indexer.util.BooleanFeatureFlagClient; import org.springframework.stereotype.Component; import static org.opengroup.osdu.indexer.config.IndexerConfigurationProperties.MAP_BOOL2STRING_FEATURE_NAME; @@ -30,9 +31,12 @@ public class SchemaConverterPropertiesConfig implements SchemaConverterConfig { @Autowired private IFeatureFlag featureFlagChecker; + @Autowired + private BooleanFeatureFlagClient partitionFlagChecker; - public SchemaConverterPropertiesConfig(IFeatureFlag flag) { + public SchemaConverterPropertiesConfig(IFeatureFlag flag, BooleanFeatureFlagClient flagClient) { if (flag != null) featureFlagChecker=flag; + if (flagClient != null) partitionFlagChecker=flagClient; skippedDefinitions = getDefaultSkippedDefinitions(); supportedArrayTypes = getDefaultSupportedArrayTypes(); specialDefinitionsMap = getDefaultSpecialDefinitionsMap(); @@ -72,7 +76,22 @@ public class SchemaConverterPropertiesConfig implements SchemaConverterConfig { private Map<String, String> getDefaultPrimitiveTypesMap() { Map<String, String> defaultPrimitiveTypesMap = new HashMap<>(); - if (this.featureFlagChecker.isFeatureEnabled(MAP_BOOL2STRING_FEATURE_NAME)) { + /* + Logic behing feature flag settings + If service-level FF is ON for a given deployment/environment, then the FF is ON for all the data partitions under the deployment/environment + If service-level FF is OFF but it is ON for a given data partition, then the FF is ON for the given data partition + + With this solution, the service providers can decide which level of FF should be turned on. For example, + If there are only few data partitions in a given deployment and there are not many data needed to be indexed, + the service provider can turn on the service-level FF and re-index all the data partitions in one shot. + If there are lots of data partitions or some data partitions have lots of data in a given deployment, step should be as following. + + 1. Turn the service-level FF to be OFF + 2. Turn on the FF via partition service for a given data partition and re-index all the data in the given data partition + 3. Repeat step 2 until all the data partitions have been re-indexed + 4. Turn the service-level FF to be ON and re-deploy the service. So all new data partitions apply the fix + */ + if (this.featureFlagChecker.isFeatureEnabled(MAP_BOOL2STRING_FEATURE_NAME) || partitionFlagChecker.isEnabled(MAP_BOOL2STRING_FEATURE_NAME, false)) { // in the earlier versions boolean was translated to bool and // this caused mapping boolean values like text as entry in StorageType entry in map is boolean // in some places boolean is still presented as bool so here both are normalized to boolean diff --git a/indexer-core/src/main/java/org/opengroup/osdu/indexer/service/StorageIndexerPayloadMapper.java b/indexer-core/src/main/java/org/opengroup/osdu/indexer/service/StorageIndexerPayloadMapper.java index 4b20fd821..a43a1078f 100644 --- a/indexer-core/src/main/java/org/opengroup/osdu/indexer/service/StorageIndexerPayloadMapper.java +++ b/indexer-core/src/main/java/org/opengroup/osdu/indexer/service/StorageIndexerPayloadMapper.java @@ -35,6 +35,7 @@ import org.opengroup.osdu.indexer.util.PropertyUtil; import org.opengroup.osdu.indexer.util.geo.decimator.DecimatedResult; import org.opengroup.osdu.indexer.util.geo.decimator.GeoShapeDecimator; import org.opengroup.osdu.indexer.util.geo.extractor.PointExtractor; +import org.opengroup.osdu.indexer.util.BooleanFeatureFlagClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -70,6 +71,9 @@ public class StorageIndexerPayloadMapper { @Autowired private IFeatureFlag featureFlagChecker; + @Autowired + private BooleanFeatureFlagClient booleanFeatureFlagClient; + public Map<String, Object> mapDataPayload(ArrayList<String> asIngestedCoordinatesPaths, IndexSchema storageSchema, Map<String, Object> storageRecordData, String recordId) { @@ -116,11 +120,26 @@ public class StorageIndexerPayloadMapper { switch (elasticType) { case KEYWORD: case TEXT: + /* + Logic behing feature flag settings + If service-level FF is ON for a given deployment/environment, then the FF is ON for all the data partitions under the deployment/environment + If service-level FF is OFF but it is ON for a given data partition, then the FF is ON for the given data partition + + With this solution, the service providers can decide which level of FF should be turned on. For example, + If there are only few data partitions in a given deployment and there are not many data needed to be indexed, + the service provider can turn on the service-level FF and re-index all the data partitions in one shot. + If there are lots of data partitions or some data partitions have lots of data in a given deployment, step should be as following. + + 1. Turn the service-level FF to be OFF + 2. Turn on the FF via partition service for a given data partition and re-index all the data in the given data partition + 3. Repeat step 2 until all the data partitions have been re-indexed + 4. Turn the service-level FF to be ON and re-deploy the service. So all new data partitions apply the fix + */ // Feature flag enforces conversion of values to string when index type is string, this is required for features using // copy_to mechanic like bag of words, however originally index type for boolean values was string so this would convert // those values to string without changing the index type, changing index type would require migration so it is also // under this feature flag as not all users will be interested with it - if (this.featureFlagChecker.isFeatureEnabled(MAP_BOOL2STRING_FEATURE_NAME)) { + if (this.featureFlagChecker.isFeatureEnabled(MAP_BOOL2STRING_FEATURE_NAME) || booleanFeatureFlagClient.isEnabled(MAP_BOOL2STRING_FEATURE_NAME, false)) { this.attributeParsingService.tryParseString(recordId, schemaPropertyName, storageRecordValue, dataCollectorMap); } else { dataCollectorMap.put(schemaPropertyName, storageRecordValue); @@ -128,7 +147,7 @@ public class StorageIndexerPayloadMapper { break; case KEYWORD_ARRAY: case TEXT_ARRAY: - if (this.featureFlagChecker.isFeatureEnabled(MAP_BOOL2STRING_FEATURE_NAME)) { + if (this.featureFlagChecker.isFeatureEnabled(MAP_BOOL2STRING_FEATURE_NAME) || booleanFeatureFlagClient.isEnabled(MAP_BOOL2STRING_FEATURE_NAME, false)) { this.attributeParsingService.tryParseValueArray(String.class, recordId, schemaPropertyName, storageRecordValue, dataCollectorMap); } else { dataCollectorMap.put(schemaPropertyName, storageRecordValue); diff --git a/indexer-core/src/test/java/org/opengroup/osdu/indexer/schema/converter/PropertiesProcessorTest.java b/indexer-core/src/test/java/org/opengroup/osdu/indexer/schema/converter/PropertiesProcessorTest.java index 90a141eb6..5700a71d3 100644 --- a/indexer-core/src/test/java/org/opengroup/osdu/indexer/schema/converter/PropertiesProcessorTest.java +++ b/indexer-core/src/test/java/org/opengroup/osdu/indexer/schema/converter/PropertiesProcessorTest.java @@ -37,6 +37,7 @@ import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; import static org.opengroup.osdu.indexer.config.IndexerConfigurationProperties.MAP_BOOL2STRING_FEATURE_NAME; import org.opengroup.osdu.core.common.feature.IFeatureFlag; +import org.opengroup.osdu.indexer.util.BooleanFeatureFlagClient; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.Import; @@ -44,9 +45,9 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringRunner; @RunWith(SpringRunner.class) -@Import({IFeatureFlag.class}) -@ContextConfiguration(classes = {PropertiesProcessorTest.class, IFeatureFlag.class}) -@SpringBootTest(classes = {IFeatureFlag.class}) +@Import({IFeatureFlag.class, BooleanFeatureFlagClient.class}) +@ContextConfiguration(classes = {PropertiesProcessorTest.class, IFeatureFlag.class, BooleanFeatureFlagClient.class}) +@SpringBootTest(classes = {IFeatureFlag.class, BooleanFeatureFlagClient.class}) public class PropertiesProcessorTest { private static final String PATH = "given_path"; @@ -55,12 +56,15 @@ public class PropertiesProcessorTest { @MockBean private IFeatureFlag featureFlagChecker; + @MockBean + private BooleanFeatureFlagClient partitionFlagClient; + private SchemaConverterPropertiesConfig schemaConverterConfig; @Before public void setup() throws IOException { initMocks(this); - schemaConverterConfig = new SchemaConverterPropertiesConfig(featureFlagChecker); + schemaConverterConfig = new SchemaConverterPropertiesConfig(featureFlagChecker, partitionFlagClient); } @Test diff --git a/indexer-core/src/test/java/org/opengroup/osdu/indexer/schema/converter/SchemaToStorageFormatImplTest.java b/indexer-core/src/test/java/org/opengroup/osdu/indexer/schema/converter/SchemaToStorageFormatImplTest.java index 07960a3db..a7769064f 100644 --- a/indexer-core/src/test/java/org/opengroup/osdu/indexer/schema/converter/SchemaToStorageFormatImplTest.java +++ b/indexer-core/src/test/java/org/opengroup/osdu/indexer/schema/converter/SchemaToStorageFormatImplTest.java @@ -27,6 +27,7 @@ import org.opengroup.osdu.core.common.logging.JaxRsDpsLog; import org.opengroup.osdu.indexer.cache.partitionsafe.VirtualPropertiesSchemaCache; import org.opengroup.osdu.indexer.schema.converter.config.SchemaConverterPropertiesConfig; import org.opengroup.osdu.indexer.schema.converter.exeption.SchemaProcessingException; +import org.opengroup.osdu.indexer.util.BooleanFeatureFlagClient; import static org.opengroup.osdu.indexer.config.IndexerConfigurationProperties.MAP_BOOL2STRING_FEATURE_NAME; import org.springframework.context.annotation.Configuration; @@ -63,13 +64,16 @@ public class SchemaToStorageFormatImplTest { private IFeatureFlag featureFlag; + private BooleanFeatureFlagClient partitionFlagClient; + private SchemaConverterPropertiesConfig schemaConverterPropertiesConfig; @Before public void init() { MockitoAnnotations.initMocks(this); featureFlag = Mockito.mock(IFeatureFlag.class); + partitionFlagClient = Mockito.mock(BooleanFeatureFlagClient.class); virtualPropertiesSchemaCache = Mockito.mock(VirtualPropertiesSchemaCache.class); - schemaConverterPropertiesConfig = new SchemaConverterPropertiesConfig(featureFlag); + schemaConverterPropertiesConfig = new SchemaConverterPropertiesConfig(featureFlag, partitionFlagClient); schemaToStorageFormatImpl = new SchemaToStorageFormatImpl(objectMapper, jaxRsDpsLog, schemaConverterPropertiesConfig, virtualPropertiesSchemaCache); diff --git a/indexer-core/src/test/java/org/opengroup/osdu/indexer/service/StorageIndexerPayloadMapperTest.java b/indexer-core/src/test/java/org/opengroup/osdu/indexer/service/StorageIndexerPayloadMapperTest.java index 80fe386da..30e229a2c 100644 --- a/indexer-core/src/test/java/org/opengroup/osdu/indexer/service/StorageIndexerPayloadMapperTest.java +++ b/indexer-core/src/test/java/org/opengroup/osdu/indexer/service/StorageIndexerPayloadMapperTest.java @@ -27,6 +27,7 @@ import org.opengroup.osdu.indexer.util.geo.decimator.GeoShapeDecimator; import org.opengroup.osdu.indexer.util.geo.decimator.GeometryDecimator; import org.opengroup.osdu.indexer.util.geo.extractor.PointExtractor; import org.opengroup.osdu.indexer.util.parser.*; +import org.opengroup.osdu.indexer.util.BooleanFeatureFlagClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; @@ -49,7 +50,7 @@ import static org.opengroup.osdu.indexer.model.Constants.AS_INGESTED_COORDINATES GeometryDecimator.class, PointExtractor.class, GeometryConversionService.class, FeatureFlagCache.class, DpsHeaders.class, JobStatus.class, SchemaConverterPropertiesConfig.class, JaxRsDpsLog.class, ServiceAccountJwtClientMock.class, VirtualPropertiesSchemaCacheMock.class, VirtualPropertiesSchemaCache.class, RequestInfoMock.class, - IFeatureFlag.class, StringParser.class} + IFeatureFlag.class, StringParser.class, BooleanFeatureFlagClient.class} ) public class StorageIndexerPayloadMapperTest { @@ -86,6 +87,9 @@ public class StorageIndexerPayloadMapperTest { @MockBean protected IFeatureFlag featureFlagChecker; + @MockBean + protected BooleanFeatureFlagClient partitionFlagChecker; + @BeforeClass public static void setUp() { HashMap<String, Object> dataMap = new HashMap<>(); -- GitLab