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


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/cdc/HoodieNativeLogFormatWriter.java:
##########
@@ -277,27 +278,41 @@ private void ensureAppendVersion() throws IOException {
   }
 
   private void closeFileWriters() throws IOException {
-    if (dataFileWriter != null) {
-      dataFileWriter.close();
-      if (writeConfig.isMetadataColumnStatsIndexEnabled()) {
-        try {
-          lastDataFileFormatMetadata = 
Option.ofNullable(dataFileWriter.getFileFormatMetadata());
-        } catch (UnsupportedOperationException e) {
-          // File-format metadata is an optional writer capability. 
Unsupported formats can still produce valid
-          // native log files; they simply do not contribute column stats for 
this append.
+    try {
+      try {
+        if (dataFileWriter != null) {
           lastDataFileFormatMetadata = Option.empty();
+          dataFileWriter.close();
+          if (writeConfig.isMetadataColumnStatsIndexEnabled()) {
+            try {
+              lastDataFileFormatMetadata = 
Option.ofNullable(dataFileWriter.getFileFormatMetadata());
+            } catch (UnsupportedOperationException e) {
+              // File-format metadata is an optional writer capability. 
Unsupported formats can still produce valid
+              // native log files; they simply do not contribute column stats 
for this append.
+              lastDataFileFormatMetadata = Option.empty();
+            }
+          }
         }
-      } else {
-        lastDataFileFormatMetadata = Option.empty();
+      } finally {
+        dataFileWriter = null;

Review Comment:
   🤖 This can happen in production, not just in the test. When a rollover 
`flushAppend()` fails inside `writeInsertAndUpdate`/`writeDelete`, 
`HoodieAppendHandle#writeRecord` catches it. If `hoodie.write.ignore.failed` is 
on (the Spark default is `true`), it marks that one record as failed and keeps 
going instead of closing the writer. Now that the writers are nulled, 
`canWriteDataFile()` returns true on the next record. `ensureAppendVersion()` 
does nothing because `currentAppendVersion` is still set, so 
`createNativeLogFile` builds the same version and write-token path as the file 
whose close failed. Resetting `currentAppendVersion`, `dataLogFile` and 
`deleteLogFile` in a `finally` in `flushAppend` (or making the writer fail fast 
after a failed flush) would avoid a collision with or overwrite of that partial 
file.



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

Reply via email to