Skip to content

fix: release BulkWriter locks in finally blocks - #2106

Merged
sre-ci-robot merged 1 commit into
milvus-io:masterfrom
waqqas-vectara:fix/bulkwriter-lock-release
Oct 6, 2026
Merged

sre-ci-robot merged 1 commit into
milvus-io:masterfrom
waqqas-vectara:fix/bulkwriter-lock-release

Conversation

@waqqas-vectara

Copy link
Copy Markdown
Contributor

Fixes #2104

appendLock, fileWriteLock and workingThreadLock were unlocked without try/finally. If the code they guard threw (an IOException or OutOfMemoryError from fileWriter.appendRow, or a failed chunk upload in commitIfFileReady), the throwing thread kept the lock. Every other thread that later called appendRow then parked in ReentrantLock.lock() forever.

Changes:

  • BulkWriter: appendRow, verifyRow, commit() and newFileWriter now unlock in finally.
  • LocalBulkWriter: callBackIfCommitReady and exit() do the same for workingThreadLock.
  • BulkWriterTest.testAppendLockReleasedWhenCommitThrows: makes commitIfFileReady throw on one thread, then checks that appendLock is free and that a second thread's appendRow gets the same exception instead of blocking. The test fails on master and passes with this change.

🤖 Generated with Claude Code

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>
@sre-ci-robot

Copy link
Copy Markdown

Welcome @waqqas-vectara! It looks like this is your first PR to milvus-io/milvus-sdk-java 🎉

@mergify

mergify Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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.

@xiaofan-luan

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@sre-ci-robot

Copy link
Copy Markdown

[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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@codecov

codecov Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@287c6b9). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...ain/java/io/milvus/bulkwriter/LocalBulkWriter.java 60.00% 2 Missing ⚠️
...src/main/java/io/milvus/bulkwriter/BulkWriter.java 92.30% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2106   +/-   ##
=========================================
  Coverage          ?   80.90%           
=========================================
  Files             ?      492           
  Lines             ?    30171           
  Branches          ?     3123           
=========================================
  Hits              ?    24411           
  Misses            ?     4449           
  Partials          ?     1311           
Files with missing lines Coverage Δ
...src/main/java/io/milvus/bulkwriter/BulkWriter.java 74.89% <92.30%> (ø)
...ain/java/io/milvus/bulkwriter/LocalBulkWriter.java 79.56% <60.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mergify mergify Bot added the ci-passed label Oct 6, 2026
@sre-ci-robot
sre-ci-robot merged commit f1c0e03 into milvus-io:master Oct 6, 2026
9 checks passed
mergify Bot pushed a commit that referenced this pull request Oct 6, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BulkWriter.appendRow leaks appendLock when an append throws, deadlocking other writer threads

3 participants