[ISSUE #9583] Re-register receipt handle after changing invisible duration#10586
[ISSUE #9583] Re-register receipt handle after changing invisible duration#10586Aias00 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a proxy-side receipt-handle lifecycle bug where changeInvisibleDuration could return a new receipt handle from the broker but fail to re-register it with the proxy’s ReceiptHandleManager when the original handle was under auto-renew management, leading to later ACK lookups failing.
Changes:
- Re-register managed
MessageReceiptHandleinstances afterchangeInvisibleTimesucceeds by updating them with the broker-returned receipt handle. - Preserve existing behavior for non-managed receipt handles (no re-registration).
- Add targeted unit tests covering both the managed and non-managed paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| proxy/src/main/java/org/apache/rocketmq/proxy/grpc/v2/consumer/ChangeInvisibleDurationActivity.java | Updates response conversion flow to update/re-add managed receipt handles using the new handle returned by the broker. |
| proxy/src/test/java/org/apache/rocketmq/proxy/grpc/v2/consumer/ChangeInvisibleDurationActivityTest.java | Adds tests asserting re-registration happens only when the old handle was managed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proxy auto-renew removes the old receipt handle before changing invisible duration. When the broker returns a new receipt handle, the proxy must put that managed handle back; otherwise later ACKs cannot find the updated handle in the receipt handle manager. Constraint: Related to apache#9583 Rejected: Recreate MessageReceiptHandle from only the new receipt handle | loses existing managed metadata such as message context and retry state Confidence: high Scope-risk: narrow Directive: Re-register only handles that were already managed by the receipt handle manager, and reuse the channel captured before the async broker callback. Tested: mvn -pl proxy -Dtest=ChangeInvisibleDurationActivityTest test Tested: mvn -pl proxy -Dtest=ReceiptHandleProcessorTest test Not-tested: Full repository test suite
Aias00
force-pushed
the
fix/change-invisible-reregister-handle
branch
from
82c768e to
ecc43e9
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #10586 +/- ##
=============================================
- Coverage 48.26% 48.17% -0.10%
+ Complexity 13433 13409 -24
=============================================
Files 1378 1378
Lines 100817 100823 +6
Branches 13040 13041 +1
=============================================
- Hits 48660 48572 -88
- Misses 46211 46289 +78
- Partials 5946 5962 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
okay, by the way, |
Which Issue(s) This PR Fixes
Fixes #9583
Brief Description
When proxy auto-renew is managing a receipt handle,
changeInvisibleDurationremoves the old handle from the receipt handle manager before callingchangeInvisibleTime. The broker returns a new receipt handle on success, but the proxy previously only returned it to the client and did not put the managed handle back intoReceiptHandleManager.This leaves the updated handle unmanaged, so later ACK requests can fail to find the receipt handle mapping.
This PR updates the removed
MessageReceiptHandlewith the new receipt handle returned by the broker and re-registers it only when the old handle was already managed. Non-managed handles keep the existing behavior.How Did You Test This Change?
mvn -pl proxy -Dtest=ChangeInvisibleDurationActivityTest testResult:
BUILD SUCCESS,Tests run: 7, Failures: 0, Errors: 0, Skipped: 0