chore | delete configs permanently rather than soft deletes (#178)

* chore | delete configs permanently rather than soft deletes
hypertrace · Oct 12, 2023 · 73e3e05 · 73e3e05
1 parent b58a082
commit 73e3e05
Show file tree

Hide file tree

Showing 11 changed files with 205 additions and 158 deletions.
diff --git a/config-service-factory/build.gradle.kts b/config-service-factory/build.gradle.kts
@@ -6,7 +6,7 @@ dependencies {
   api(libs.hypertrace.grpc.framework)
 
   implementation(projects.configServiceChangeEventGenerator)
-
+  implementation(libs.hypertrace.documentstore)
   implementation(projects.configServiceImpl)
   implementation(projects.spacesConfigServiceImpl)
   implementation(projects.labelsConfigServiceImpl)

diff --git a/config-service-factory/src/main/java/org/hypertrace/config/service/ConfigServiceFactory.java b/config-service-factory/src/main/java/org/hypertrace/config/service/ConfigServiceFactory.java
@@ -14,6 +14,8 @@
 import org.hypertrace.config.service.change.event.impl.ConfigChangeEventGeneratorFactory;
 import org.hypertrace.config.service.store.ConfigStore;
 import org.hypertrace.config.service.store.DocumentConfigStore;
+import org.hypertrace.core.documentstore.Datastore;
+import org.hypertrace.core.documentstore.DatastoreProvider;
 import org.hypertrace.core.serviceframework.grpc.GrpcPlatformService;
 import org.hypertrace.core.serviceframework.grpc.GrpcPlatformServiceFactory;
 import org.hypertrace.core.serviceframework.grpc.GrpcServiceContainerEnvironment;
@@ -28,6 +30,8 @@ public class ConfigServiceFactory implements GrpcPlatformServiceFactory {
   private static final String SERVICE_NAME = "config-service";
   private static final String STORE_REPORTING_NAME = "config-service-store";
   private static final String GENERIC_CONFIG_SERVICE_CONFIG = "generic.config.service";
+  private static final String DOC_STORE_CONFIG_KEY = "document.store";
+  private static final String DATA_STORE_TYPE = "dataStoreType";
 
   private ConfigStore store;
   private GrpcServiceContainerEnvironment grpcServiceContainerEnvironment;
@@ -86,12 +90,19 @@ protected ConfigChangeEventGenerator buildChangeEventGenerator(Config config) {
 
   protected ConfigStore buildConfigStore(Config config) {
     try {
-      ConfigStore configStore = new DocumentConfigStore();
-      configStore.init(config.getConfig(GENERIC_CONFIG_SERVICE_CONFIG));
+      Datastore datastore = initDataStore(config.getConfig(GENERIC_CONFIG_SERVICE_CONFIG));
+      ConfigStore configStore = new DocumentConfigStore(Clock.systemUTC(), datastore);
       this.store = configStore;
       return configStore;
     } catch (Exception e) {
       throw new RuntimeException("Error in getting or initializing config store", e);
     }
   }
+
+  private Datastore initDataStore(Config config) {
+    Config docStoreConfig = config.getConfig(DOC_STORE_CONFIG_KEY);
+    String dataStoreType = docStoreConfig.getString(DATA_STORE_TYPE);
+    Config dataStoreConfig = docStoreConfig.getConfig(dataStoreType);
+    return DatastoreProvider.getDatastore(dataStoreType, dataStoreConfig);
+  }
 }
diff --git a/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceGrpcImpl.java b/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceGrpcImpl.java
@@ -1,6 +1,6 @@
 package org.hypertrace.config.service;
 
-import static org.hypertrace.config.service.ConfigServiceUtils.emptyValue;
+import static org.hypertrace.config.service.ConfigServiceUtils.emptyConfig;
 import static org.hypertrace.config.service.ConfigServiceUtils.filterNull;
 import static org.hypertrace.config.service.ConfigServiceUtils.merge;
 
@@ -11,6 +11,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
 import org.hypertrace.config.service.store.ConfigStore;
@@ -69,14 +71,21 @@ public void upsertConfig(
   public void getConfig(
       GetConfigRequest request, StreamObserver<GetConfigResponse> responseObserver) {
     try {
-      ContextSpecificConfig config = configStore.getConfig(getConfigResourceContext(request));
-
+      ConfigResourceContext configResourceContext = getConfigResourceContext(request);
+      ContextSpecificConfig config =
+          configStore
+              .getConfig(configResourceContext)
+              .orElse(emptyConfig(configResourceContext.getContext()));
       // get the configs for the contexts mentioned in the request and merge them in the specified
       // order
       for (String context : request.getContextsList()) {
-        ContextSpecificConfig contextSpecificConfig =
+        Optional<ContextSpecificConfig> maybeContextConfig =
             configStore.getConfig(getConfigResourceContext(request, context));
-        config = merge(config, contextSpecificConfig);
+        ContextSpecificConfig lastConfig = config;
+        config =
+            maybeContextConfig
+                .map(contextConfig -> merge(lastConfig, contextConfig))
+                .orElse(config);
       }
 
       filterNull(config)
@@ -122,21 +131,20 @@ public void getAllConfigs(
   @Override
   public void deleteConfig(
       DeleteConfigRequest request, StreamObserver<DeleteConfigResponse> responseObserver) {
+
     try {
       ConfigResourceContext configResourceContext = getConfigResourceContext(request);
-      ContextSpecificConfig configToDelete = configStore.getConfig(configResourceContext);
-
-      // if configToDelete is null/empty (i.e. config value doesn't exist or is already deleted),
-      // then throw NOT_FOUND exception
-      if (ConfigServiceUtils.isNull(configToDelete.getConfig())) {
-        responseObserver.onError(Status.NOT_FOUND.asException());
-        return;
-      }
+      DeleteConfigResponse deleteResponse =
+          configStore
+              .getConfig(configResourceContext)
+              .map(
+                  configToDelete ->
+                      DeleteConfigResponse.newBuilder().setDeletedConfig(configToDelete).build())
+              .orElseThrow(Status.NOT_FOUND::asException);
 
-      // write an empty config for the specified config resource. This maintains the versioning.
-      configStore.writeConfig(configResourceContext, getUserId(), emptyValue());
-      responseObserver.onNext(
-          DeleteConfigResponse.newBuilder().setDeletedConfig(configToDelete).build());
+      // delete the config for the specified config resource.
+      configStore.deleteConfigs(Set.of(configResourceContext));
+      responseObserver.onNext(deleteResponse);
       responseObserver.onCompleted();
     } catch (Exception e) {
       log.error("Delete config failed for request:{}", request, e);
@@ -155,39 +163,24 @@ public void deleteConfigs(
                 .asException());
         return;
       }
-      Map<ConfigResourceContext, Value> valuesByContext =
-          request.getConfigsList().stream()
-              .map(
-                  requestedDelete ->
-                      Map.entry(this.getConfigResourceContext(requestedDelete), emptyValue()))
-              .collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue));
-
-      List<UpsertedConfig> deletedConfigs =
-          configStore.writeAllConfigs(valuesByContext, getUserId());
 
+      List<ConfigResourceContext> configResourceContexts =
+          request.getConfigsList().stream()
+              .map(this::getConfigResourceContext)
+              .collect(Collectors.toUnmodifiableList());
+      Map<ConfigResourceContext, ContextSpecificConfig> configs =
+          configStore.getContextConfigs(configResourceContexts);
+      // delete the configs for the specified config resources.
+      configStore.deleteConfigs(configs.keySet());
       responseObserver.onNext(
-          DeleteConfigsResponse.newBuilder()
-              .addAllDeletedConfigs(
-                  deletedConfigs.stream()
-                      .map(this::buildDeletedContextSpecificConfig)
-                      .collect(Collectors.toUnmodifiableList()))
-              .build());
+          DeleteConfigsResponse.newBuilder().addAllDeletedConfigs(configs.values()).build());
       responseObserver.onCompleted();
     } catch (Exception e) {
       log.error("Delete configs failed for request: {}", request, e);
       responseObserver.onError(e);
     }
   }
 
-  private ContextSpecificConfig buildDeletedContextSpecificConfig(UpsertedConfig deletedConfig) {
-    return ContextSpecificConfig.newBuilder()
-        .setContext(deletedConfig.getContext())
-        .setCreationTimestamp(deletedConfig.getCreationTimestamp())
-        .setUpdateTimestamp(deletedConfig.getUpdateTimestamp())
-        .setConfig(deletedConfig.getPrevConfig())
-        .build();
-  }
-
   @Override
   public void upsertAllConfigs(
       UpsertAllConfigsRequest request, StreamObserver<UpsertAllConfigsResponse> responseObserver) {

diff --git a/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceUtils.java b/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceUtils.java
@@ -31,6 +31,7 @@ private ConfigServiceUtils() {
   public static ContextSpecificConfig merge(
       ContextSpecificConfig defaultContextSpecificConfig,
       ContextSpecificConfig overridingContextSpecificConfig) {
+
     Value defaultConfig = defaultContextSpecificConfig.getConfig();
     Value overridingConfig = overridingContextSpecificConfig.getConfig();
     if (isNull(defaultConfig)) {

diff --git a/config-service-impl/src/main/java/org/hypertrace/config/service/store/ConfigStore.java b/config-service-impl/src/main/java/org/hypertrace/config/service/store/ConfigStore.java
@@ -1,10 +1,11 @@
 package org.hypertrace.config.service.store;
 
 import com.google.protobuf.Value;
-import com.typesafe.config.Config;
 import java.io.IOException;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import org.hypertrace.config.service.ConfigResource;
 import org.hypertrace.config.service.ConfigResourceContext;
 import org.hypertrace.config.service.v1.ContextSpecificConfig;
@@ -15,14 +16,6 @@
  * resources.
  */
 public interface ConfigStore {
-
-  /**
-   * Initialize the config store
-   *
-   * @param config
-   */
-  void init(Config config);
-
   /**
    * Write the config value associated with the specified config resource to the store.
    *
@@ -40,7 +33,19 @@ UpsertedConfig writeConfig(
    * @param configResourceContext
    * @return
    */
-  ContextSpecificConfig getConfig(ConfigResourceContext configResourceContext) throws IOException;
+  Optional<ContextSpecificConfig> getConfig(ConfigResourceContext configResourceContext)
+      throws IOException;
+
+  /**
+   * Get all the configs with the latest version(along with the context to which it applies) for the
+   * specified resource keys
+   *
+   * @param configResourceContexts
+   * @return the configs
+   * @throws IOException
+   */
+  Map<ConfigResourceContext, ContextSpecificConfig> getContextConfigs(
+      Collection<ConfigResourceContext> configResourceContexts) throws IOException;
 
   /**
    * Get all the configs with the latest version(along with the context to which it applies) for the
@@ -63,6 +68,13 @@ UpsertedConfig writeConfig(
   List<UpsertedConfig> writeAllConfigs(
       Map<ConfigResourceContext, Value> resourceContextValueMap, String userId) throws IOException;
 
+  /**
+   * delete the config values associated with the specified config resources.
+   *
+   * @param configResourceContexts
+   */
+  void deleteConfigs(Collection<ConfigResourceContext> configResourceContexts) throws IOException;
+
   /**
    * Health check for the backend store
    *