Aias00 commented on PR #10585: URL: https://github.com/apache/rocketmq/pull/10585#issuecomment-4947134554
Thanks for the review. Addressed in `8f6a9c9b`: The missing-settings fallback no longer uses `Settings.getDefaultInstance()` (whose `backoffPolicy.maxAttempts == 0`). It now falls back to the real consumer default via the new `GrpcClientSettingsManager#getDefaultConsumerSettings()`, which reuses the existing `createDefaultConsumerSettingsBuilder()` path — the same default already applied on the normal `updateClientSettings` path — yielding `maxAttempts = retryMaxTimes + 1` (17 with the default `SubscriptionGroupConfig`). So a fresh message (`reconsumeTimes == 0`) is no longer routed to the DLQ by `PopMessageResultFilterImpl`. Did not change `getClientSettings()` to return the default instead of `null`, since `ClientActivity#heartbeat` relies on `null` to reply `UNRECOGNIZED_CLIENT_TYPE`; the fallback stays local to the receive path. Added regression coverage: - `PopMessageResultFilterImplTest` — fresh message (`reconsumeTimes == 0`) is `MATCH` under the real default, and documents that `maxAttempts == 0` DLQs it. - `GrpcClientSettingsManagerTest#testDefaultConsumerSettingsHasNonZeroMaxAttempts` — default settings carry a positive `maxAttempts`. - `ReceiveMessageActivityTest#testReceiveMessageWithMissingClientSettings` — captures the `PopMessageResultFilterImpl` built on the missing-settings path and asserts a fresh message is not `TO_DLQ`. `mvn -pl proxy -Dtest=ReceiveMessageActivityTest,PopMessageResultFilterImplTest,GrpcClientSettingsManagerTest test` → 21 passed, 0 failed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
