[I] [Bug] A failing page invalidation abandons the lookup store's file handle two levels up [paimon]

["PDGGK (via GitHub)" <gi...@apache.org>]

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: issues-unsubscribe@paimon.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug] A failing page invalidation abandons the lookup store's file handle two levels up [paimon]

["JingsongLi (via GitHub)" <gi...@apache.org>]

JingsongLi closed issue #9171: [Bug] A failing page invalidation abandons the lookup store's file handle two levels up
URL: https://github.com/apache/paimon/issues/9171


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org