LuciferYang opened a new pull request, #9636:
URL: https://github.com/apache/paimon/pull/9636
### Purpose
close #9635
`FileIndexFormat.Reader`'s constructor closed the stream it was handed only
for an `IOException`, while a header it does not recognize is reported with
`throw new RuntimeException("This file is not file index file.")`. Every call
site opens the stream inside the try-with-resources head:
```java
try (FileIndexFormat.Reader indexReader =
FileIndexFormat.createReader(fileIO.newInputStream(...),
schemaInfo.fileSchema)) {
```
A constructor that throws never assigns that resource, so the stream stays
open. #9462 fixed this for `FileIndexPredicate` by catching `RuntimeException`
at the call site; `FileIndexProcessor` and `FileIndexesTable` still leak, one
per failed read, and `FileIndexProcessor` runs per data file during compaction
and `rewrite-file-index`.
The `catch` in the constructor now covers `RuntimeException` too, which is
where the stream reference lives, so all three call sites are covered and the
next one does not have to remember. The call-site workaround in
`FileIndexPredicate` goes away with it.
One behavior change worth knowing about: a bad magic or version now arrives
wrapped, `RuntimeException("Exception happens while construct file index
reader.")` with the original as its cause, rather than bare.
`FileIndexesTable.fileIndexReadException` tests `getCause() instanceof
IOException` and is unaffected; the only place that asserted the old message
was `FileIndexPredicateCloseTest`, updated here.
### Tests
`FileIndexFormatFormatTest.testCreateReaderClosesStreamOnBadMagic` hands
`createReader` a stream over bytes that are not an index file, with `close()`
instrumented, and asserts the stream was closed before it asserts anything
about the exception. Ordering it that way matters: the close is what the fix is
about, and asserting the message first would let a change that only wraps the
exception pass.
`FileIndexPredicateCloseTest` keeps covering the same thing one layer up,
with its message expectation updated.
Against the unfixed constructor the new test fails on the close assertion.
`mvn -pl paimon-common
-Dtest=FileIndexFormatFormatTest,FileIndexPredicateCloseTest test` on JDK 8: 5
tests, 0 failures. `spotless:check` and `checkstyle:check` on paimon-common are
clean.
--
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]
