[GitHub] [hudi] nsivabalan commented on a change in pull request #3590: [HUDI-2285][HUDI-2476] Metadata table synchronous design. Rebased and Squashed from pull/3426

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

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



##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -241,13 +242,16 @@ void emitCommitMetrics(String instantTime, HoodieCommitMetadata metadata, String
     }
   }
 
+  /**
+   * Any pre-commit actions like conflict resolution or updating metadata table goes here.
+   * @param instantTime commit instant time.
+   * @param metadata commit metadata for which pre commit is being invoked.
+   */
   protected void preCommit(String instantTime, HoodieCommitMetadata metadata) {
-    // no-op
-    // TODO : Conflict resolution is not supported for Flink & Java engines
-  }
-
-  protected void syncTableMetadata() {
-    // no-op
+    // Create a Hoodie table after startTxn which encapsulated the commits and files visible.
+    // Important to create this after the lock to ensure latest commits show up in the timeline without need for reload

Review comment:
       I feel adding this comment at the caller does not fit in well. This is what it looks like.
   ```
    try {
         // Precommit is meant for resolving conflicts if any and to update metadata table if enabled.
         // Create a Hoodie table within preCommit after starting the transaction which encapsulated the commits and files visible 
         // (instead of using an existing instance of Hoodie table). It is important to create this after the lock to ensure latest 
         // commits show up in the timeline without need for reload.
         preCommit(instantTime, metadata);
         commit(table, commitActionType, instantTime, metadata, stats);
         postCommit(table, metadata, instantTime, extraMetadata);
         ...
   ```
   I feel having this within preCommit just belongs to the place where it is needed or used. 




-- 
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