PDGGK opened a new issue, #9171:
URL: https://github.com/apache/paimon/issues/9171

   ### Search before asking
   
   - [x] I searched in the [issues](https://github.com/apache/paimon/issues) 
and found nothing similar.
   
   ### Paimon version
   
   master
   
   ### Compute Engine
   
   Any — this is the local lookup store used by lookup joins and changelog 
production.
   
   ### Minimal reproduce step
   
   Code inspection, plus a unit test that drives a real store with a failing 
`CacheManager`.
   
   ### What doesn't meet your expectations?
   
   Closing a sorted lookup store walks three levels, and each one was a bare 
sequence of calls:
   
   | level | |
   |---|---|
   | `SortLookupStoreReader.close():83` | `reader.close()` → `input.close()` |
   | `SstFileReader.close():171` | `bloomFilter.close()` → `blockCache.close()` 
|
   | `BlockCache.close():87` | `cacheManager.invalidPage(key)` for each cached 
page, in a loop |
   
   A single failing page invalidation at the bottom therefore abandons the 
remaining pages, then the block cache, then **the file handle two levels up** — 
`input` is a `SeekableInputStream`.
   
   **The descriptor is never reclaimed, and the failure is nearly silent.** 
Both callers deliberately swallow it:
   
   ```java
   // LocalKvDb#closeAndDeleteSstFile
   try { reader.close(); } catch (IOException e) {
       LOG.warn("Failed to close reader for SST file: {}", file.getName(), e);
   }
   ```
   
   and the shutdown loop in `LocalKvDb#close` does the same. That is the right 
call — one bad reader should not stall shutdown — but it means an abandoned 
descriptor shows up only as a warning line, and nothing ever retries it. 
Readers are cached per SST file in `readerCache`, so this accumulates.
   
   `BlockCache.close()` compounds it: the pages it fails to hand back stay 
resident in a `CacheManager` that is shared across readers, with nothing left 
holding a reference that could invalidate them later.
   
   ### Anything else?
   
   This is the same shape as #9162 (`SortMergeReaderWithMinHeap.close()`), but 
a different code path — that one is the merge-tree compaction reader, this one 
is the lookup store.
   
   `IOUtils.closeAll` does not fit any of the three: it declares `throws 
Exception`, and all three of these override or implement a `close() throws 
IOException`. `ExceptionUtils.firstOrSuppressed` is the right tool and its 
javadoc already gives this close-loop as its example.
   
   ### Are you willing to submit a PR?
   
   - [x] I'm willing to submit a PR!
   


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