Re: [PR] fix(nbcc): reject insert overwrite combined with non-blocking concurrency control [hudi]

["hudi-agent (via GitHub)" <gi...@apache.org>]

hudi-agent commented on code in PR #19791:
URL: https://github.com/apache/hudi/pull/19791#discussion_r3901145682


##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/configuration/OptionsResolver.java:
##########
@@ -449,6 +449,20 @@ public static boolean isInsertOverwrite(Configuration conf) {
         || conf.get(FlinkOptions.OPERATION).equalsIgnoreCase(WriteOperationType.INSERT_OVERWRITE.value());
   }
 
+  /**
+   * Validates that insert overwrite is not combined with non-blocking concurrency control.
+   *
+   * <p>Insert overwrite reuses the deterministic bucket file id under non-blocking concurrency
+   * control, but the replace commit it generates records that same file id as replaced. Since the
+   * file system view hides a replaced file group by file id (ignoring the replace instant), the
+   * freshly overwritten data would become invisible. Reject the combination to avoid data loss.
+   *
+   */
+  public static void checkNonBlockingConcurrencyControl(Configuration conf) {
+    WriteConcurrencyMode.checkInsertOverwriteSupported(
+        isNonBlockingConcurrencyControl(conf), isInsertOverwrite(conf));
+  }
+
   /**
    * Returns whether the operation is INSERT OVERWRITE dynamic partition.
    */

Review Comment:
   🤖 nit: could you rename this to something like `checkInsertOverwriteSupported(Configuration conf)` to mirror the underlying `WriteConcurrencyMode` method? `checkNonBlockingConcurrencyControl` reads more like a predicate on the mode itself and doesn't hint at what the check is actually validating.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>



##########
hudi-common/src/main/java/org/apache/hudi/common/model/WriteConcurrencyMode.java:
##########
@@ -42,9 +43,13 @@ public enum WriteConcurrencyMode {
   // Multiple writer can perform write ops on a MOR table with non-blocking conflict resolution
   @EnumFieldDescription("Multiple writers can operate on the table with non-blocking conflict resolution. "
       + "The writers can write into the same file group with the conflicts resolved automatically "
-      + "by the query reader and the compactor.")
+      + "by the query reader and the compactor. Insert overwrite is not supported in this mode.")
   NON_BLOCKING_CONCURRENCY_CONTROL;
 
+  //Error message thrown when insert overwrite is combined with non-blocking concurrency control.

Review Comment:
   🤖 nit: missing space after `//` — `//Error message` should be `// Error message` to match the surrounding comment style.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>



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