PDGGK opened a new pull request, #9163:
URL: https://github.com/apache/paimon/pull/9163

   ### Purpose
   
   Closes #9162.
   
   `SortMergeReaderWithMinHeap.close()` releases one `RecordReader` per sorted 
run being merged, across three loops, and every call was a bare `close()`. The 
first one to throw abandoned the rest of its loop **and both loops after it**.
   
   This is not a two-resource teardown: there is one reader per sorted run, 
each holds an open data file, and it runs on the compaction path. A single 
unreadable file therefore strands every descriptor behind it for the remainder 
of the merge. `element.iterator.releaseBatch()` had the same exposure — no 
checked exception, but it can still throw, and when it did the `close()` on the 
next line was skipped.
   
   Each step now runs, the first failure propagates, and later ones are 
attached to it as suppressed.
   
   **On the choice of helper.** `IOUtils.closeAll` is the obvious candidate and 
I did not use it: it declares `throws Exception`, and both existing callers 
(`FormatTableFileWriter:93`, `FileChannelManagerImpl:126`) widened their own 
signature to match. This method overrides `RecordReader.close() throws 
IOException` and cannot widen. `ExceptionUtils.firstOrSuppressed` is the right 
fit — its javadoc gives this exact close-loop as its worked example.
   
   `SortMergeReaderWithLoserTree` needs no change; it delegates to a single 
`loserTree.close()`.
   
   ### Tests
   
   New `SortMergeReaderWithMinHeapCloseTest`, three cases:
   
   | case | asserts |
   |---|---|
   | an earlier reader fails to close | all three readers still closed; the 
**first** failure propagates with the later one as suppressed |
   | `releaseBatch()` throws | both readers still closed, and the release 
failure is what the caller sees |
   | everything closes cleanly | no exception — the control |
   
   Against the previous `close()`, the first and third of those fail; the 
all-succeed control passes on **both** versions, which is what shows the other 
two aren't trivially red.
   
   The readers are a small hand-rolled fake rather than mocks, so the test 
asserts on real call ordering through `readBatch()` into the heap rather than 
on stub interactions.
   
   `spotless:check`, `checkstyle:check` and `apache-rat:check` are clean.
   
   **Local run caveat.** With `-Pfast-build`, 
`CompactionChangelogFollowUpScannerTest` and `MergeTreeCompactManagerTest` 
error in setup with `Could not instantiate generated class 'RecordComparator'` 
— the codegen module isn't built under that profile. I confirmed it's unrelated 
by reverting `SortMergeReaderWithMinHeap.java` to master and deleting the new 
test: both fail identically. Everything else in the merge-tree and compaction 
packages passes, including `MergeTreeReadersConnectionsLeakTest` (20).
   


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