[ISSUE #10559] Support wildcard pattern matching for LiteTopic subscriptions#10607
[ISSUE #10559] Support wildcard pattern matching for LiteTopic subscriptions#10607Aias00 wants to merge 4 commits into
Conversation
…subscriptions Add NATS-style wildcard pattern matching so a wildcard LiteTopic consumer group can subscribe to a subset of lite-topics under its parent topic, using "__" as the segment separator. - LitePatternMatcher (new, common): validate/matches/matchesAny/expand. "*" matches one segment; "**" matches one or more trailing segments. Recursive, no regex; internal pre-split for the expand hot path. - LiteSubscription: add in-memory wildcardPatterns set. - LiteSubscriptionRegistry[Impl]: a wildcard group with non-empty validated patterns enters pattern mode — eagerly expand against collectByParentTopic and register real lmqNames. Empty patterns keep legacy receive-all behavior (synthetic topic@group key) unchanged. getAllSubscriber always merges the synthetic-key clients for any wildcard group so mixed pattern/legacy groups deliver to both; reexpandWildcardPatterns picks up lite-topics created after the initial subscription. - LiteSubscriptionCtlProcessor: route wildcard COMPLETE_ADD through toWildcardPatterns (patterns ride the existing liteTopicSet — no wire change). Non-empty but all-invalid pattern input is rejected instead of silently widening to receive-all. - LiteEventDispatcher.doFullDispatchForWildcardGroup: replace the O(N) forEachLiteTopic scan with O(M) collectByParentTopic; re-expand patterns for new LMQs and clear backlog per pattern-mode client. Full backward compatibility: wildcard groups without patterns behave identically to before. Affected lite tests pass; checkstyle clean. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds NATS-style wildcard pattern matching for LiteTopic wildcard consumer groups, enabling clients to subscribe to a subset of lite-topics under a parent topic (using __-delimited segments) while preserving legacy “receive all” behavior when no patterns are provided. It also optimizes wildcard-group full dispatch by iterating only lite-topics under the bound parent topic.
Changes:
- Introduces
LitePatternMatcher(common) withvalidate,matches,matchesAny, andexpand, plus comprehensive unit tests. - Extends lite subscription/registry flow to support “pattern mode” wildcard groups by storing patterns and eagerly/lazily expanding them to concrete LMQ names.
- Updates broker control/dispatch paths and tests to route wildcard
COMPLETE_ADDthrough pattern handling and to avoid full LMQ table scans.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| common/src/main/java/org/apache/rocketmq/common/lite/LitePatternMatcher.java | Adds wildcard pattern validation/matching/expansion for lite-topic child names. |
| common/src/test/java/org/apache/rocketmq/common/lite/LitePatternMatcherTest.java | Tests pattern semantics, validation rules, and expansion behavior. |
| common/src/main/java/org/apache/rocketmq/common/lite/LiteSubscription.java | Adds in-memory wildcardPatterns storage for pattern-mode wildcard groups. |
| broker/src/main/java/org/apache/rocketmq/broker/processor/LiteSubscriptionCtlProcessor.java | Routes wildcard COMPLETE_ADD to registry overload carrying validated patterns. |
| broker/src/test/java/org/apache/rocketmq/broker/processor/LiteSubscriptionCtlProcessorTest.java | Adds tests for wildcard routing and all-invalid-pattern rejection. |
| broker/src/main/java/org/apache/rocketmq/broker/lite/LiteSubscriptionRegistry.java | Adds registry overload + reexpandWildcardPatterns API for pattern-mode support. |
| broker/src/main/java/org/apache/rocketmq/broker/lite/LiteSubscriptionRegistryImpl.java | Implements pattern-mode expansion, mixed legacy+pattern delivery merging, and re-expansion. |
| broker/src/test/java/org/apache/rocketmq/broker/lite/LiteSubscriptionRegistryImplTest.java | Adds tests for pattern-mode expansion/resubscribe, re-expansion, and mixed-group delivery. |
| broker/src/main/java/org/apache/rocketmq/broker/lite/LiteEventDispatcher.java | Optimizes wildcard full dispatch via collectByParentTopic and adds pattern-client re-expansion. |
| broker/src/test/java/org/apache/rocketmq/broker/lite/LiteEventDispatcherTest.java | Adds tests covering legacy vs pattern-mode wildcard full dispatch paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if (isWildcardGroup) { | ||
| // Legacy wildcard group: receive all lite-topics under the parent topic via the | ||
| // synthetic topic@group key. | ||
| lmqNameNew = Collections.singleton(mockLmqNameForWildcardGroup(topic, group)); | ||
| markWildcardGroup(topic, group); |
| if (LiteMetadataUtil.isWildcardGroup(group, brokerController)) { | ||
| // For a wildcard group, liteTopicSet carries pattern strings (not literal | ||
| // lmqNames); pass them through verbatim and let the registry expand them. | ||
| Set<String> wildcardPatterns = toWildcardPatterns(entry); | ||
| this.liteSubscriptionRegistry.addCompleteSubscription(clientId, group, topic, lmqNameSet, | ||
| wildcardPatterns, entry.getVersion()); | ||
| } else { | ||
| this.liteSubscriptionRegistry.addCompleteSubscription(clientId, group, topic, lmqNameSet, | ||
| entry.getVersion()); | ||
| } |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #10607 +/- ##
=============================================
- Coverage 48.27% 48.25% -0.02%
- Complexity 13436 13503 +67
=============================================
Files 1378 1379 +1
Lines 100820 101027 +207
Branches 13041 13105 +64
=============================================
+ Hits 48670 48755 +85
- Misses 46208 46288 +80
- Partials 5942 5984 +42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot
Summary
This PR adds NATS-style wildcard pattern matching (*, **) for LiteTopic wildcard consumer groups, enabling selective subscription to a subset of lite-topics under a parent topic. It also optimizes the wildcard-group full dispatch from O(N) full LMQ table scan to O(M) parent-topic-scoped iteration. The implementation is well-structured with 18 new unit tests and preserves backward compatibility.
Findings
-
[Warning]
LitePatternMatcher.java—matches()callsvalidate()on every invocation. Inexpand(), this means the same pattern is re-validated for every candidate lite-topic. Consider splitting into an internalmatchesUnchecked()and callingvalidate()once at the call site (e.g., inexpand()or at pattern registration time). The PR description claims ~1µs/call, but eliminating redundant validation would improve throughput in hot dispatch paths. -
[Warning]
LiteSubscriptionRegistryImpl.java:151(aligns with Copilot) — When a client transitions from pattern-mode back to legacy mode (empty patterns) or to a non-wildcard group, the stalewildcardPatternsfield onLiteSubscriptionis not cleared. This can causedoFullDispatchForWildcardGroupto misclassify the client as pattern-mode andreexpandWildcardPatternsto re-register stale LMQs. Suggested fix: explicitly callsetWildcardPatterns(Collections.emptySet())in the non-pattern branch ofaddCompleteSubscription. -
[Warning]
LiteSubscriptionRegistryImpl.java:401-413—doFullDispatchForWildcardGroupfirst callsgetAllClientIdByGroup(group)then iterates to check each client's subscription. Between these two calls, a subscription could be removed or changed. Consider snapshotting the client list and subscription state under a single lock scope, or documenting that the broker event loop guarantees single-threaded access here. -
[Info]
LitePatternMatcher.java:120-137—expand()splits the pattern into segments viapattern.split(SEPARATOR, -1)on every call. For patterns reused across many candidates, pre-splitting once and passing the segments array would avoid repeated allocations. -
[Info]
LiteSubscriptionRegistryImpl.java:430-457—reexpandWildcardPatternsacquires and releases the read lock once per client. If many pattern-mode clients share the same parent topic, consider batching the expansion under a single lock acquisition to reduce lock contention. -
[Info]
LiteSubscriptionCtlProcessor.java:107(aligns with Copilot) — ThelmqNameSetderived from pattern strings viaLiteUtil.toLmqName()is semantically misleading since these are patterns, not actual lite-topic names. The set is passed to the registry but appears unused for pattern-mode groups. Consider passingCollections.emptySet()ornullfor thelmqNameAllparameter when in pattern mode to make the intent clearer.
Suggestions
- Add a
matchesUnchecked(String[] patternSegments, String liteTopic)package-private variant to avoid redundantvalidate()+split()in tight loops. - Clear
wildcardPatternson mode transition to prevent stale state bugs. - The cross-repo impact is minimal since patterns ride the existing
liteTopicSetwire format — no changes needed inrocketmq-clients. 👍
Overall
Solid implementation with good test coverage and clean backward compatibility. The main concern is the stale wildcardPatterns on mode transition (shared with Copilot's finding). The repeated validate() calls are a performance optimization opportunity.
Automated review by github-manager-bot
…topics Address review findings on the wildcard pattern matching feature (apache#10559): 1. Pattern→legacy/non-wildcard transition left stale wildcardPatterns on the LiteSubscription, so doFullDispatchForWildcardGroup / reexpandWildcardPatterns (which key off a non-empty wildcardPatterns set) kept misclassifying the client as pattern-mode. Now clear wildcardPatterns in both the legacy-wildcard and non-wildcard branches of addCompleteSubscription. 2. A pattern-mode client was not delivered a newly-created matching lite-topic on the normal message-arriving dispatch path — it only got picked up by the periodic doFullDispatchForWildcardGroup re-expand, causing delivery delay/loss. Add LiteSubscriptionRegistry#registerArrivingLmqForPatternClients(lmqName): on the message-arriving path (group == null) it enumerates the parent topic's pattern-mode clients, matches their stored patterns against the single arriving lmqName's child (no O(M) collectByParentTopic scan), and registers matches via addTopicGroup so the immediately-following getAllSubscriber finds them. The periodic re-expand remains as a backstop. 3. Nit: wildcard COMPLETE_ADD now passes Collections.emptySet() for lmqNameSet (pattern-mode registration ignores it; lmqNames are derived by expanding patterns). CI note: bazel-compile BrokerShutdownTest TIMEOUT and macOS ServiceThreadTest stress flake are pre-existing and unrelated (neither test is in this diff nor touches lite/wildcard code). Co-Authored-By: Claude <noreply@anthropic.com>
…ern expansion paths The maxLiteSubscriptionCount quota gate existed only in addPartialSubscription. The wildcard pattern paths added in apache#10559 — addCompleteSubscription (pattern-mode and non-wildcard), the periodic reexpandWildcardPatterns, and the message-arriving registerArrivingLmqForPatternClients — all call addTopicGroup and increment activeNum with no quota check, so a wide pattern could expand subscription references without bound at runtime, bypassing the broker quota. Close all three paths with context-appropriate enforcement: - addCompleteSubscription: pre-flight throw (LiteQuotaException, already mapped to LITE_SUBSCRIPTION_QUOTA_EXCEEDED by the processor's existing catch) projecting net-new references against the limit, before any mutation. - reexpandWildcardPatterns / registerArrivingLmqForPatternClients: cap and log (no throw — these run on the dispatcher background thread / message-arriving hot path with no client to respond to); register up to the limit, then stop. The guard precedes addLiteTopic so the subscription's LiteTopicSet stays consistent with liteTopic2Group on early exit. Added 4 tests covering both throw paths and both cap paths. Co-Authored-By: Claude <noreply@anthropic.com>
… removals Two gaps in addCompleteSubscription's quota gate (follow-up apache#3): 1. State leak on quota failure. The check ran AFTER setWildcardPatterns and markWildcardGroup, so a quota-exceeded throw left non-empty wildcardPatterns plus a wildcardGroupMap entry. reexpandWildcardPatterns and registerArrivingLmqForPatternClients (which key off non-empty patterns / the group map) then picked up the failed client and turned it into a deferred-effect subscription. 2. wouldAdd only counted gross additions, not crediting the lmqNames the same operation would remove. A net-zero replace at the quota limit (drop one lmq, add another) was falsely rejected. Fix: compute lmqNameNew (expand is read-only) BEFORE any mutation, project wouldAdd as max(0, added - removed), run checkQuotaOrThrow first, then apply setWildcardPatterns / markWildcardGroup / add-remove. A quota failure now leaves only the harmless empty placeholder LiteSubscription — no patterns, no group mark, no active refs. Tests: strengthened the pattern quota test to assert no pattern/group state leaks; added net-zero-replace-at-quota (must pass) and a pattern-quota-failure deferred-state test asserting later re-expand / arriving-lmq register nothing for the rejected client. Co-Authored-By: Claude <noreply@anthropic.com>
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot (Re-review)
Summary
Three new commits since the initial review address dispatch gaps for newly-created lite-topics, add subscription quota enforcement, and fix quota projection ordering. The implementation is solid with comprehensive test coverage.
Changes Since Last Review
✅ Dispatch gap for new lite-topics — FIXED (a26ffbb5): Added registerArrivingLmqForPatternClients() which lazily registers an arriving lmqName against pattern-mode wildcard clients whose patterns match it. This closes the race window between lite-topic creation and the periodic doFullDispatchForWildcardGroup backstop. The addTopicGroup now returns boolean to track actual new entries — clean design.
✅ Subscription quota enforcement — ADDED (0e92110e + 2c597641): checkQuotaOrThrow() pre-flight check is now applied to all registration paths (non-wildcard complete-add, wildcard pattern expansion, and registerArrivingLmqForPatternClients). The quota projection was correctly moved before mutation in commit 2c597641 to avoid partial state on rejection. Good defensive programming.
✅ Mixed pattern/legacy wildcard groups — HANDLED: getAllSubscriber() now merges synthetic-key clients for legacy wildcard groups with pattern-mode clients. doFullDispatchForWildcardGroup partitions clients into pattern-mode and legacy subsets, dispatching each appropriately.
✅ Tests — EXCELLENT: New tests cover quota rejection on all paths, mixed-group delivery, deferred state cleanup on quota failure, and the lazy registration path. 138+ tests all green.
Addressed from Previous Review
- [Warning]
expand()pre-splitting — ✅ Now usescompilePattern()to pre-split once and reuses the segments array across all candidates. - [Warning] Stale
wildcardPatterns— ✅ ThedeferredStateCleanup()helper now explicitly clearswildcardPatternswhen transitioning out of pattern mode. - [Info] Lock contention in
reexpandWildcardPatterns— Partially addressed: the per-client expansion is now more targeted (only matching patterns), reducing the work under each lock acquisition.
Remaining Items (Non-blocking)
-
[Warning]
matches()still callsvalidate()on every invocation. Whileexpand()now pre-splits, directmatches()callers (e.g., inregisterArrivingLmqForPatternClients) still pay the validation cost per call. Consider caching validated patterns at registration time. -
[Info] The
#separator used inLiteQuotaExceptionmessages is fine for logging but should not be used as a key delimiter in any future data structure (topics can contain#in some configurations).
Verdict
The three new commits significantly strengthen the implementation. Dispatch gaps are closed, quota is enforced consistently, and the code is well-tested. LGTM — ready for committer review.
Automated re-review by github-manager-bot
Which issue does this PR close?
Closes #10559.
What's the purpose of this PR
Add NATS-style wildcard pattern matching so a wildcard LiteTopic consumer group can subscribe to a subset of lite-topics under its parent topic (instead of always receiving all of them), using
__as the segment separator. Full backward compatibility is preserved: wildcard groups without patterns behave identically to before.Brief changelog
LitePatternMatcher(new,common) — the matching engine:validate,matches(recursive, no regex),matchesAny,expand.*matches exactly one__-delimited segment;**matches one or more trailing segments (like NATS>).LiteSubscription— add in-memorywildcardPatternsset (rebuilt on reconnect, same asliteTopicSet).LiteSubscriptionRegistry[Impl]— a wildcard group with non-empty validated patterns enters pattern mode: eagerly expand againstcollectByParentTopicand register real lmqNames. Empty patterns keep legacy receive-all behavior (synthetictopic@groupkey) unchanged.getAllSubscriberalways merges the synthetic-key clients for any wildcard group, so mixed pattern/legacy groups deliver to both. NewreexpandWildcardPatternspicks up lite-topics created after the initial subscription.LiteSubscriptionCtlProcessor— route wildcardCOMPLETE_ADDthroughtoWildcardPatterns(patterns ride the existingliteTopicSet— no wire protocol change). Non-empty but all-invalid pattern input is rejected instead of silently widening to receive-all.LiteEventDispatcher.doFullDispatchForWildcardGroup— replace the O(N)forEachLiteTopicscan with O(M)collectByParentTopic; re-expand patterns for new LMQs and clear backlog per pattern-mode client.Pattern semantics
pay__refundpay__refundpay__refund__notify,notify__refundpay__*pay__refund,pay__successpay__refund__notify*__refundpay__refund,notify__refundpay__refund__notifypay__*__notifypay__refund__notify,pay__success__notifypay__refund**pay__**pay__refund,pay__refund__notifynotify__refundHow to test / verify
LitePatternMatcherTest(common): all six issue rows (match + non-match),validateaccept/reject cases,expand/matchesAnysemantics — 18 tests.mvn -pl common,broker -am test— lite suite green (138 tests, 0 failures); checkstyle clean on common + broker.matches≈ 1µs/call (target <5µs);expand(1k candidates × 5 patterns)≈ 360µs (target <10ms).Scope
Broker + common only, matching the issue's "Key Changes" table. Patterns are carried via the existing
LiteSubscriptionDTO.liteTopicSet; the broker distinguishes them viaisWildcardGroup(). A consumer/proxy API (subscribeLite) and a true LMQ-creation listener for sub-second new-topic pickup are intentionally deferred to follow-up PRs.🤖 Generated with Claude Code