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


##########
hudi-common/src/main/java/org/apache/hudi/common/table/read/lsm/LsmFileGroupRecordIterator.java:
##########
@@ -153,28 +154,37 @@ public LsmFileGroupRecordIterator(HoodieReaderContext<T> 
readerContext,
    */
   private List<SortedRunReader<T>> initializeReaders() throws IOException {
     List<SortedRunReader<T>> sortedRunReaders = new ArrayList<>();
-    int mergeOrder = 0;
-    if (readBaseFile) {
-      addReader(sortedRunReaders, mergeOrder++, 
LsmFileIterators.createBaseFileIterator(
-          readerContext, storage, inputSplit.getBaseFileOption().get(),
-          inputSplit.getStart(), inputSplit.getLength(), orderingFieldNames, 
false));
-    }
+    try {
+      int mergeOrder = 0;
+      if (readBaseFile) {
+        addReader(sortedRunReaders, mergeOrder++, 
LsmFileIterators.createBaseFileIterator(
+            readerContext, storage, inputSplit.getBaseFileOption().get(),
+            inputSplit.getStart(), inputSplit.getLength(), orderingFieldNames, 
false));
+      }
 
-    if (inputSplit.hasRecordIterator()) {
-      addReader(sortedRunReaders, mergeOrder++, 
createRecordIterator(inputSplit.getRecordIterator()));
-    }
+      if (inputSplit.hasRecordIterator()) {
+        addReader(sortedRunReaders, mergeOrder++, 
createRecordIterator(inputSplit.getRecordIterator()));
+      }
 
-    List<LogReaderSpec> logReaderSpecs = new ArrayList<>();
-    if (!inputSplit.hasRecordIterator()) {
-      for (HoodieLogFile logFile : inputSplit.getLogFiles()) {
-        logReaderSpecs.add(new LogReaderSpec(mergeOrder++, logFile));
+      List<LogReaderSpec> logReaderSpecs = new ArrayList<>();
+      if (!inputSplit.hasRecordIterator()) {
+        for (HoodieLogFile logFile : inputSplit.getLogFiles()) {
+          logReaderSpecs.add(new LogReaderSpec(mergeOrder++, logFile));
+        }
       }
-    }
-    Set<Integer> directLogMergeOrders = 
selectDirectLogMergeOrders(logReaderSpecs, readBaseFile);
-    for (LogReaderSpec spec : logReaderSpecs) {
-      ClosableIterator<BufferedRecord<T>> iterator = 
LsmFileIterators.createLogFileIterator(
-          readerContext, metaClient, storage, spec.logFile, 
orderingFieldNames);
-      addReader(sortedRunReaders, spec.mergeOrder, 
maybeSpillIterator(directLogMergeOrders.contains(spec.mergeOrder), iterator));
+      Set<Integer> directLogMergeOrders = 
selectDirectLogMergeOrders(logReaderSpecs, readBaseFile);
+      for (LogReaderSpec spec : logReaderSpecs) {
+        ClosableIterator<BufferedRecord<T>> iterator = 
LsmFileIterators.createLogFileIterator(
+            readerContext, metaClient, storage, spec.logFile, 
orderingFieldNames);
+        addReader(sortedRunReaders, spec.mergeOrder, 
maybeSpillIterator(directLogMergeOrders.contains(spec.mergeOrder), iterator));

Review Comment:
   🤖 I don't think anything gets swallowed here — the block ends with `throw 
e;`, so an `Error` still propagates after cleanup, just with any close failures 
attached as suppressed (Java's precise rethrow keeps the `throws IOException` 
signature valid). Narrowing to `Exception` would actually skip the cleanup on 
OOM, which is arguably the case where releasing spill files matters most, and 
the inline comment on line 178 already states that intent.



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