Skip to content

[ISSUE #10894] Enforce Lite subscription quota for complete and batch updates - #10919

Open
btlqql wants to merge 1 commit into
apache:developfrom
btlqql:btlqql/fix-lite-subscription-quota
Open

[ISSUE #10894] Enforce Lite subscription quota for complete and batch updates#10919
btlqql wants to merge 1 commit into
apache:developfrom
btlqql:btlqql/fix-lite-subscription-quota

Conversation

@btlqql

@btlqql btlqql commented Aug 12, 2026

Copy link
Copy Markdown

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

  • LiteSubscriptionRegistryImpl.addPartialSubscription: check the quota per newly added lmqName, skipping entries the client already subscribes to
  • LiteSubscriptionRegistryImpl.addCompleteSubscription: apply the same per-entry quota check
  • added isLiteTopicSubscribed() helper that returns whether a client group is already registered for an lmqName, so idempotent retries are not rejected

How was this patch verified

  • Code review of the quota arithmetic around addTopicGroup/removeTopicGroup
  • git diff --check clean

@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

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 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.

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, the maxCount variable 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, maxCount is fetched from brokerController.getBrokerConfig() inside the loop. Consider caching it outside the loop to avoid repeated config lookups.
  • [Info] The new isLiteTopicSubscribed helper 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

@btlqql
btlqql force-pushed the btlqql/fix-lite-subscription-quota branch from 0be6904 to e696527 Compare August 13, 2026 05:59

@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.

LGTM. Trivial change, looks good.


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.

[Bug] Enforce Lite subscription quota for complete and batch updates

2 participants