[ISSUE #10894] Enforce Lite subscription quota for complete and batch updates - #10919
[ISSUE #10894] Enforce Lite subscription quota for complete and batch updates#10919btlqql wants to merge 1 commit into
Conversation
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Summary
This PR adds quota enforcement for Lite subscriptions in LiteSubscriptionRegistryImpl. Both addPartialSubscription and addCompleteSubscription now check the active subscription count against maxLiteSubscriptionCount before adding new topics. A new helper method isLiteTopicSubscribed() is added to avoid counting already-subscribed topics against the quota.
The quota enforcement is correctly placed before the actual subscription addition, and the check properly excludes topics that are already subscribed (re-subscriptions don't count against quota).
LGTM. Good fix.
Note: This PR also includes changes from #10914-#10918. See the series overlap comment on those PRs.
Automated review by github-manager-bot
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review Summary
This PR enforces the Lite subscription quota limit for both partial and complete subscription updates, preventing clients from exceeding the configured maxLiteSubscriptionCount.
Findings
- [Warning] LiteSubscriptionRegistryImpl.java:97 — In
addPartialSubscription, themaxCountvariable is referenced but not visible in the diff context. Please verify it's accessible as a field or method parameter. - [Warning] LiteSubscriptionRegistryImpl.java:150 — In
addCompleteSubscription,maxCountis fetched frombrokerController.getBrokerConfig()inside the loop. Consider caching it outside the loop to avoid repeated config lookups. - [Info] The new
isLiteTopicSubscribedhelper method correctly checks if a topic is already subscribed by a client group, preventing false quota violations on re-subscription.
Overall
Good defensive fix to enforce subscription limits. The quota check prevents resource exhaustion from clients subscribing to too many topics. Minor optimization opportunity with config caching.
Automated review by RockteMQ-AI
0be6904 to
e696527
Compare
RockteMQ-AI
left a comment
There was a problem hiding this comment.
LGTM. Trivial change, looks good.
Automated review by github-manager-bot
What is the purpose of the change
Fix #10894.
LiteSubscriptionRegistryImpl.addPartialSubscription checked only the current active count before processing, so a request adding multiple active Lite subscriptions could exceed maxLiteSubscriptionCount. addCompleteSubscription did not check the limit at all. At the same time an idempotent retry for an already active subscription was rejected once the quota was full.
Brief changelog
How was this patch verified
git diff --checkclean