You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Aias00 (via GitHub)" <gi...@apache.org> on 2026/07/29 05:54:13 UTC
[PR] [ISSUE #10681] Skip lite offline cleanup without subscriptions [rocketmq]
Aias00 opened a new pull request, #10682:
URL: https://github.com/apache/rocketmq/pull/10682
### Summary
- Skip lite-client offline cleanup when settings have no subscription section or no subscription entries.
- Avoid indexing `subscriptions(0)` for incomplete lite-consumer settings.
- Add a regression test for lite consumers without subscription entries.
### Motivation
Closes #10681.
`GrpcClientSettingsManager.offlineClientLiteSubscription` previously assumed lite-consumer settings always contain at least one subscription entry. Incomplete settings could throw `IndexOutOfBoundsException` and fall into the broad error log path, including the full settings object.
### Tests
- `mvn -pl proxy -Dtest=GrpcClientSettingsManagerTest -DfailIfNoTests=false test`
Result: BUILD SUCCESS; 8 tests passed; checkstyle reported 0 violations. The run emits existing JaCoCo instrumentation warnings under the local JDK, and one existing test intentionally logs a simulated sync failure, but tests and build completed successfully.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10681] Skip lite offline cleanup without subscriptions [rocketmq]
Posted by "f1amingo (via GitHub)" <gi...@apache.org>.
f1amingo commented on PR #10682:
URL: https://github.com/apache/rocketmq/pull/10682#issuecomment-5128310534
Thanks for the PR. However, this guards a state that cannot occur by design.
In the liteTopic model, the bind topic is mandatory on the SDK side — a lite consumer always carries exactly one bind-topic entry in its Settings, established before any lite subscription exists. So `getSubscriptions(0)` is safe here by contract.
If this state ever appeared, it would signal a protocol violation that should fail loudly (which the existing catch + error log already does), not be silently skipped on the offline path.
I'd suggest walking through the end-to-end liteTopic subscription flow (Settings sync → bind topic → SyncLiteSubscription) first — it would make it clear why this invariant holds.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10681] Skip lite offline cleanup without subscriptions [rocketmq]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 closed pull request #10682: [ISSUE #10681] Skip lite offline cleanup without subscriptions
URL: https://github.com/apache/rocketmq/pull/10682
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10681] Skip lite offline cleanup without subscriptions [rocketmq]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #10682:
URL: https://github.com/apache/rocketmq/pull/10682#issuecomment-5138150874
> Thanks for the PR. However, this guards a state that cannot occur by design.
>
> In the liteTopic model, the bind topic is mandatory on the SDK side — a lite consumer always carries exactly one bind-topic entry in its Settings, established before any lite subscription exists. So `getSubscriptions(0)` is safe here by contract.
>
> If this state ever appeared, it would signal a protocol violation that should fail loudly (which the existing catch + error log already does), not be silently skipped on the offline path.
>
> I'd suggest walking through the end-to-end liteTopic subscription flow (Settings sync → bind topic → SyncLiteSubscription) first — it would make it clear why this invariant holds.
sounds good to me
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10681] Skip lite offline cleanup without subscriptions [rocketmq]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10682:
URL: https://github.com/apache/rocketmq/pull/10682#issuecomment-5113988559
## [Codecov](https://app.codecov.io/gh/apache/rocketmq/pull/10682?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `50.00000%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 48.23%. Comparing base ([`00e45b8`](https://app.codecov.io/gh/apache/rocketmq/commit/00e45b8a6db23efbe756d0306f10716156cfd4dd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`46c9474`](https://app.codecov.io/gh/apache/rocketmq/commit/46c9474466a6acfa12d6498a411ae2ca5803a123?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/rocketmq/pull/10682?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...roxy/grpc/v2/common/GrpcClientSettingsManager.java](https://app.codecov.io/gh/apache/rocketmq/pull/10682?src=pr&el=tree&filepath=proxy%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Frocketmq%2Fproxy%2Fgrpc%2Fv2%2Fcommon%2FGrpcClientSettingsManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJveHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Byb3h5L2dycGMvdjIvY29tbW9uL0dycGNDbGllbnRTZXR0aW5nc01hbmFnZXIuamF2YQ==) | 50.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/rocketmq/pull/10682?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #10682 +/- ##
=============================================
- Coverage 48.31% 48.23% -0.09%
+ Complexity 13511 13485 -26
=============================================
Files 1380 1380
Lines 101091 101093 +2
Branches 13101 13102 +1
=============================================
- Hits 48844 48759 -85
- Misses 46285 46346 +61
- Partials 5962 5988 +26
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/rocketmq/pull/10682?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org