hudi-agent commented on code in PR #19791:
URL: https://github.com/apache/hudi/pull/19791#discussion_r3988809093
##########
hudi-common/src/main/java/org/apache/hudi/common/model/WriteConcurrencyMode.java:
##########
@@ -62,6 +67,21 @@ public boolean isNonBlockingConcurrencyControl() {
}
public static boolean isNonBlockingConcurrencyControl(String name) {
- return
WriteConcurrencyMode.valueOf(name.toUpperCase()).isNonBlockingConcurrencyControl();
+ return
WriteConcurrencyMode.valueOf(name.toUpperCase(Locale.ROOT)).isNonBlockingConcurrencyControl();
+ }
+
+ /**
+ * Rejects the {@code insert overwrite} + non-blocking concurrency control
(NB-CC) combination,
+ * which silently loses data.
+ *
+ * @param isNonBlockingConcurrencyControl whether the write concurrency mode
is NB-CC
+ * @param isOverwrite whether the operation is an
insert overwrite
+ * (see {@link
WriteOperationType#isOverwrite})
+ * @throws HoodieException if the combination is requested
+ */
Review Comment:
🤖 nit: the extra alignment spaces before `isOverwrite` in the @param list
look like leftover formatting from an editor — worth normalizing to a single
space like the line above.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/configuration/OptionsResolver.java:
##########
@@ -463,6 +463,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 checkInsertOverwriteSupported(Configuration conf) {
+ WriteConcurrencyMode.checkInsertOverwriteSupported(
+ isNonBlockingConcurrencyControl(conf), isInsertOverwrite(conf));
Review Comment:
🤖 nit: there's a stray blank javadoc line before the closing `*/` here —
could you remove it?
<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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
