PDGGK opened a new pull request, #9407:
URL: https://github.com/apache/paimon/pull/9407
### Purpose
`BTreeIndexReader` opens a `SeekableInputStream` in its constructor and then
reads the footer, the bloom filter handle and the index block through it:
```java
this.input = fileReader.getInputStream(globalIndexIOMeta);
long fileSize = globalIndexIOMeta.fileSize();
BlockCache blockCache = new BlockCache(filePath, input, cacheManager);
BTreeFileFooter footer = readFooter(blockCache, fileSize); // can throw
FileBasedBloomFilter bloomFilter = FileBasedBloomFilter.create(...); //
can throw
```
If any of those fails the constructor never returns, so nothing else ever
holds a reference to `input` and the file handle is stranded. The simplest way
in is a corrupted index file: `BTreeFileFooter.readFooter` checks a magic
number and throws `IllegalArgumentException("File is not a btree index file
(bad magic number)")`.
`BitmapIndexReader` already guards exactly this sequence, so this makes the
two readers agree:
```java
this.input = fileReader.getInputStream(meta);
try {
...
} catch (IOException | RuntimeException e) {
IOUtils.closeQuietly(input);
throw e;
}
```
`close()` had the same gap in the other direction. It was a bare sequence:
```java
reader.close();
input.close();
```
so a reader that fails to close takes the file handle with it. Both are now
closed through `IOUtils.closeAll`, which closes everything and reports the
first failure with the rest suppressed. The catch rethrows `IOException` and
`RuntimeException` unchanged rather than wrapping them, so a caller still sees
the failure the reader actually produced.
### Tests
`BTreeIndexReaderCloseTest`, three cases over a real index file written by
`BTreeGlobalIndexer`, with the input stream wrapped so closes are counted:
- `testCloseReleasesTheInput` — control: a healthy file keeps the handle
open, and `close()` releases it.
- `testFailedConstructionReleasesTheInput` — the four magic-number bytes are
overwritten on disk; construction fails and the handle is still released.
- `testCloseReleasesTheInputWhenTheReaderFails` — page invalidation is made
to fail, which is what closing the reader's bloom filter does; the reader's own
exception comes out unchanged and the handle is still released.
Reverting the change on a clean rebuild turns the two failure cases red on
the close counter (`Expecting AtomicInteger(0) to have value: 1`) and leaves
the control green.
`mvn test -pl paimon-common
-Dtest='org.apache.paimon.globalindex.**,org.apache.paimon.sst.**,org.apache.paimon.lookup.**'`
— 425 tests, all passing.
--
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]
