Copilot commented on code in PR #10607:
URL: https://github.com/apache/rocketmq/pull/10607#discussion_r3556479562
##########
broker/src/main/java/org/apache/rocketmq/broker/lite/LiteSubscriptionRegistryImpl.java:
##########
@@ -124,8 +126,27 @@ public void removePartialSubscription(String clientId,
String group, String topi
@Override
public void addCompleteSubscription(String clientId, String group, String
topic, Set<String> lmqNameAll, long version) {
+ addCompleteSubscription(clientId, group, topic, lmqNameAll,
Collections.emptySet(), version);
+ }
+
+ @Override
+ public void addCompleteSubscription(String clientId, String group, String
topic, Set<String> lmqNameAll,
+ Set<String> wildcardPatterns, long version) {
Set<String> lmqNameNew;
- if (LiteMetadataUtil.isWildcardGroup(group, brokerController)) {
+ boolean isWildcardGroup = LiteMetadataUtil.isWildcardGroup(group,
brokerController);
+ boolean isPatternMode = isWildcardGroup && wildcardPatterns != null &&
!wildcardPatterns.isEmpty();
+
+ LiteSubscription thisSub = getOrCreateLiteSubscription(clientId,
group, topic);
+ if (isPatternMode) {
+ // Persist the patterns (authoritative intent) and eagerly expand
against existing
+ // lite-topics under the parent topic. The expanded lmqNames are
registered as a normal
+ // subscription; the topic@group synthetic key is NOT used for
pattern-mode groups.
+ thisSub.setWildcardPatterns(wildcardPatterns);
+ lmqNameNew = expandWildcardPatterns(topic, wildcardPatterns);
+ markWildcardGroup(topic, group);
+ } 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);
Review Comment:
`addCompleteSubscription` only calls `setWildcardPatterns(...)` in
pattern-mode. If a client previously subscribed in pattern mode and later
re-subscribes in legacy mode (empty patterns) or as a non-wildcard group, the
old `wildcardPatterns` remain on the `LiteSubscription`. This can misclassify
the client as pattern-mode (e.g., `doFullDispatchForWildcardGroup` will treat
it as pattern-mode) and can also cause `reexpandWildcardPatterns` to
re-register matched LMQs unexpectedly.
##########
broker/src/main/java/org/apache/rocketmq/broker/processor/LiteSubscriptionCtlProcessor.java:
##########
@@ -94,8 +95,16 @@ public RemotingCommand processRequest(ChannelHandlerContext
ctx, RemotingCommand
case COMPLETE_ADD:
checkConsumeEnable(group);
this.liteSubscriptionRegistry.updateClientChannel(clientId, ctx.channel());
-
this.liteSubscriptionRegistry.addCompleteSubscription(clientId, group, topic,
lmqNameSet,
- entry.getVersion());
+ 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());
+ }
Review Comment:
In the wildcard `COMPLETE_ADD` path, `lmqNameSet` is derived by converting
the incoming `liteTopicSet` entries with `LiteUtil.toLmqName(...)`. But for
wildcard groups, `liteTopicSet` carries wildcard pattern strings and should not
be treated as literal child topic names. Passing a pattern-derived `lmqNameSet`
into the registry is confusing and makes it easy to accidentally start relying
on invalid LMQ names in future changes.
--
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]
