Skip to content

fix(proxy): clamp pop result filter maxAttempts to at least one attempt - #11140

Open
zjncs wants to merge 1 commit into
apache:developfrom
zjncs:fix/pop-filter-zero-max-attempts
Open

zjncs wants to merge 1 commit into
apache:developfrom
zjncs:fix/pop-filter-zero-max-attempts

Conversation

@zjncs

@zjncs zjncs commented Sep 11, 2026

Copy link
Copy Markdown

Motivation

receiveMessage feeds settings.getBackoffPolicy().getMaxAttempts() into PopMessageResultFilterImpl. When a client leaves the backoff policy unset, proto3 defaults maxAttempts to 0, and while the subscription group config is unavailable (mergeSubscriptionData passes the raw client settings through on a config miss) every popped message — including its first delivery — hit reconsumeTimes >= maxAttempts and was routed straight to the DLQ.

Modifications

Clamp maxAttempts to at least 1 in the filter, so a message always gets one delivery attempt before the DLQ verdict.

Verification

Fail-before (new test, run against the unpatched code):

PopMessageResultFilterImplTest#testZeroMaxAttemptsStillDeliversFirstAttempt
java.lang.AssertionError: expected:<MATCH> but was:<TO_DLQ>
  PopMessageResultFilterImplTest.testZeroMaxAttemptsStillDeliversFirstAttempt:47

Pass-after:

Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 -- PopMessageResultFilterImplTest

receiveMessage feeds settings.getBackoffPolicy().getMaxAttempts() into
PopMessageResultFilterImpl. When a client leaves the backoff policy
unset, proto3 defaults maxAttempts to 0, and while the subscription
group config is unavailable (mergeSubscriptionData passes the raw
client settings through on a config miss) every popped message,
including its first delivery, hit reconsumeTimes >= 0 and was routed
straight to the DLQ.

Clamp maxAttempts to at least 1 in the filter so a message always gets
one delivery attempt before the DLQ verdict.
Copilot AI lite review requested due to automatic review settings September 11, 2026 07:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Clamp maxAttempts to at least 1 in PopMessageResultFilterImpl, ensuring that clients with proto3 default (0) backoff policy still get at least one delivery attempt before messages go to the DLQ.

LGTM — correct proto3 edge case handling with thorough test.


Automated review by github-manager-bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants