[fix][offload] Close all resources in BlobStoreBackedReadHandleImplV2.closeAsync#25296
Conversation
|
@iamsanjaymalakar Please add the following content to your PR description and select a checkbox: |
codelipenghui
left a comment
There was a problem hiding this comment.
And could you please also add tests to cover the changes from this PR, which can confirm the issue can be reproduced and avoid regression in the future.
|
Thanks for the reviews! I addressed the feedback by making |
|
Please run "Personal CI" to validate such changes on your side. You can run Pulsar CI in your personal fork by following these instructions: https://pulsar.apache.org/contribute/personal-ci/. It's free. |
|
One detail about BlobStoreBackedReadHandleImplV2 is that it's not used in Pulsar's production code. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #25296 +/- ##
============================================
- Coverage 72.80% 72.61% -0.19%
+ Complexity 34660 34198 -462
============================================
Files 1968 1968
Lines 155954 156134 +180
Branches 17766 17787 +21
============================================
- Hits 113537 113383 -154
- Misses 33447 33695 +248
- Partials 8970 9056 +86
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
….closeAsync (apache#25296) (cherry picked from commit c83cafc) (cherry picked from commit e190cb5)
….closeAsync (apache#25296) (cherry picked from commit c83cafc) (cherry picked from commit e190cb5)
….closeAsync (apache#25296) (cherry picked from commit c83cafc) (cherry picked from commit e190cb5)
….closeAsync (apache#25296) (cherry picked from commit c83cafc) (cherry picked from commit e190cb5)
Motivation
BlobStoreBackedReadHandleImplV2#closeAsync()closes indices and dataStreams sequentially inside a single try/catch. If closing one resource throws, the method exits early and the remaining resources are never closed. This can leak file descriptors and leave resources open when offload read handles are closed.Modifications
Update
closeAsync()to attempt closing allOffloadIndexBlockV2instances and allDataInputStreams.Collect the first
IOExceptionand attach subsequent failures via suppressed exceptions.Complete the returned
CompletableFutureexceptionally if any close operation fails; otherwise mark the handle Closed and complete normally.docdoc-requireddoc-not-neededdoc-complete