Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RocketMq retry-topic support SQL filtering messages #1750

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

chengyouling
Copy link
Collaborator

What type of PR is this?

Bug.

What this PR does / why we need it?

Retry-topic synchronous adjustment change to using SQL92 filter message.

Which issue(s) this PR fixes?

Fixes #1748

Does this PR introduce a user-facing change?

No

Checklist

  • Make sure there is a GitHub_issue related with this PR before you start working on it.
  • Make sure you have squashed your change to one single commit.
  • GitHub Actions works fine in this PR.

Copy link

codecov bot commented Feb 24, 2025

Codecov Report

Attention: Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...cketMqSchedulerRebuildSubscriptionInterceptor.java 84.09% 4 Missing and 3 partials ⚠️
Flag Coverage Δ Complexity Δ
unittests 27.75% <85.10%> (+0.05%) 156.00 <0.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ Complexity Δ
...grayscale/rocketmq/utils/RocketMqReflectUtils.java 50.00% <ø> (ø) 0.00 <0.00> (ø)
.../rocketmq/utils/RocketMqSubscriptionDataUtils.java 68.21% <100.00%> (ø) 0.00 <0.00> (ø)
...cketMqSchedulerRebuildSubscriptionInterceptor.java 80.70% <84.09%> (+12.95%) 0.00 <0.00> (ø)

Comment on lines 50 to 63
if (balance.getConsumerGroup() == null || !isGrayTagChanged(map, balance)) {
return context;
}
for (Object subscriptionData : map.values()) {
String topic = RocketMqReflectUtils.getTopic(subscriptionData);
if (topic.contains(RETYPE)) {
continue;
}
if (RocketMqSubscriptionDataUtils
.isExpressionTypeInaccurate(RocketMqReflectUtils.getExpressionType(subscriptionData))) {
continue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isGrayTagChanged has the similar code with these code. You can traverse the map only once

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines 82 to 100
private void updateRetrySubscriptionData(Object subscriptionData, Collection<Object> subscriptionDatas) {
String originTopic = RocketMqReflectUtils.getTopic(subscriptionData);
for (Object subData : subscriptionDatas) {
String retryTopic = RocketMqReflectUtils.getTopic(subData);
String sqlSubstr = RocketMqReflectUtils.getSubString(subscriptionData);
if (retryTopic.contains(RETYPE)) {
RocketMqReflectUtils.getTagsSet(subData).clear();
RocketMqReflectUtils.getCodeSet(subData).clear();
RocketMqReflectUtils.setSubscriptionDatae(subData, "setExpressionType",
new Class[]{String.class}, new Object[]{"SQL92"});
RocketMqReflectUtils.setSubscriptionDatae(subData, "setSubVersion",
new Class[]{long.class}, new Object[]{System.currentTimeMillis()});
String originSubData = RocketMqReflectUtils.getSubString(subData);
RocketMqReflectUtils.setSubscriptionDatae(subData, "setSubString",
new Class[]{String.class}, new Object[]{sqlSubstr});
LOGGER.warning(String.format(Locale.ENGLISH, "update retry topic [%s] SQL92 expression, "
+ "originTopic: [%s], originSubStr: [%s], newSubStr: [%s]", retryTopic, originTopic,
originSubData, sqlSubstr));
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why traverse map.values() and determine whether the topic contains again here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@chengyouling chengyouling force-pushed the develop-retry branch 2 times, most recently from b7316e5 to 768cf5c Compare February 25, 2025 01:54
RocketMqConsumerGroupAutoCheck.setMqClientInstance(topic, consumerGroup, instance);
RocketMqConsumerGroupAutoCheck.syncUpdateCacheGrayTags();
RocketMqConsumerGroupAutoCheck.startSchedulerCheckGroupTask();
private boolean isGrayTagChanged(ConcurrentMap<String, Object> map, RebalanceImpl balance,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not map,use specific name

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@chengyouling chengyouling force-pushed the develop-retry branch 2 times, most recently from cd7a266 to e0054b4 Compare February 25, 2025 07:24
for (Object subData : retrySubscriptionDatas) {
RocketMqReflectUtils.getTagsSet(subData).clear();
RocketMqReflectUtils.getCodeSet(subData).clear();
RocketMqReflectUtils.setSubscriptionDatae(subData, "setExpressionType",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename setSubscriptionDatae

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

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.

RocketMq retry-topic support SQL filtering messages
3 participants