[GitHub] [hudi] nsivabalan commented on a change in pull request #3820: [HUDI-2579] Make deltastreamer checkpoint state merging more explicit

[GitBox <gi...@apache.org>]

nsivabalan commented on a change in pull request #3820:
URL: https://github.com/apache/hudi/pull/3820#discussion_r733011813



##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java
##########
@@ -111,16 +118,27 @@
     }
   }
 
-  // override the current metadata with the metadata from the latest instant for the specified key prefixes
-  private static void overrideWithLatestCommitMetadata(HoodieTableMetaClient metaClient, Option<HoodieCommitMetadata> thisMetadata,
-      Option<HoodieInstant> thisInstant, List<String> keyPrefixes) {
+  private static void mergeCheckpointStateFromPreviousCommit(HoodieTableMetaClient metaClient, Option<HoodieCommitMetadata> thisMetadata) {
+    overrideWithLatestCommitMetadata(metaClient, thisMetadata, Collections.singletonList(HoodieWriteConfig.DELTASTREAMER_CHECKPOINT_KEY));
+  }
+
+  /**
+   * Generic method allowing us to override the current metadata with the metadata from
+   * the latest instant for the specified key prefixes
+   * @param metaClient
+   * @param thisMetadata
+   * @param keyPrefixes The key prefixes to merge from the previous commit
+   */
+  private static void overrideWithLatestCommitMetadata(HoodieTableMetaClient metaClient,

Review comment:
       I don't see a need for this method. we can just embed within mergeCheckpointStateFromPreviousCommit. basically, rename overrideWithLatestCommitMetadata to mergeCheckpointStateFromPreviousCommit.

##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java
##########
@@ -368,11 +372,13 @@
           + "OPTIMISTIC_CONCURRENCY_CONTROL: Multiple writers can operate on the table and exactly one of them succeed "
           + "if a conflict (writes affect the same file group) is detected.");
 
-  public static final ConfigProperty<String> WRITE_META_KEY_PREFIXES = ConfigProperty
-      .key("hoodie.write.meta.key.prefixes")
-      .defaultValue("")
-      .withDocumentation("Comma separated metadata key prefixes to override from latest commit "
-          + "during overlapping commits via multi writing");
+  public static final ConfigProperty<Boolean> WRITE_CONCURRENCY_MERGE_DELTASTREAMER_STATE = ConfigProperty
+          .key("hoodie.write.concurrency.merge.deltastreamer.state")
+          .defaultValue(false)
+          .withDocumentation("If enabled, this writer will merge Deltastreamer state "
+                  + "from the previous checkpoint in order to allow both realtime "
+                  + "and batch writers to ingest into a single table. "
+                  + "This should not be enabled on Deltastreamer writers.");

Review comment:
       do you think we can add in a validation withiin Deltastreamer code base that this config is not enabled. would be good to tighten it and leave it to end user. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org