Repository navigation
fix: release BulkWriter locks in finally blocks - #2106
Conversation
appendLock, fileWriteLock and workingThreadLock were released without try/finally. If the guarded code threw (IOException, OutOfMemoryError, a failed chunk upload in commitIfFileReady), the lock stayed held by the throwing thread, and every other thread calling appendRow parked forever. Fixes milvus-io#2104 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Waqqas Jabbar <waqqas@vectara.com>
|
Welcome @waqqas-vectara! It looks like this is your first PR to milvus-io/milvus-sdk-java 🎉 |
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: waqqas-vectara, xiaofan-luan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2106 +/- ##
=========================================
Coverage ? 80.90%
=========================================
Files ? 492
Lines ? 30171
Branches ? 3123
=========================================
Hits ? 24411
Misses ? 4449
Partials ? 1311
🚀 New features to boost your workflow:
|
appendLock, fileWriteLock and workingThreadLock were released without try/finally. If the guarded code threw (IOException, OutOfMemoryError, a failed chunk upload in commitIfFileReady), the lock stayed held by the throwing thread, and every other thread calling appendRow parked forever. Fixes #2104 (cherry picked from commit f1c0e03) Signed-off-by: Waqqas Jabbar <waqqas@vectara.com> Co-authored-by: Waqqas Jabbar <waqqas@vectara.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Fixes #2104
appendLock,fileWriteLockandworkingThreadLockwere unlocked withouttry/finally. If the code they guard threw (anIOExceptionorOutOfMemoryErrorfromfileWriter.appendRow, or a failed chunk upload incommitIfFileReady), the throwing thread kept the lock. Every other thread that later calledappendRowthen parked inReentrantLock.lock()forever.Changes:
BulkWriter:appendRow,verifyRow,commit()andnewFileWriternow unlock infinally.LocalBulkWriter:callBackIfCommitReadyandexit()do the same forworkingThreadLock.BulkWriterTest.testAppendLockReleasedWhenCommitThrows: makescommitIfFileReadythrow on one thread, then checks thatappendLockis free and that a second thread'sappendRowgets the same exception instead of blocking. The test fails onmasterand passes with this change.🤖 Generated with Claude Code