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.

Sign up for GitHub

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

[ISSUE #8963] Fix code36 request sent to ns #8964

Merged
merged 2 commits into from
Nov 27, 2024
Merged

[ISSUE #8963] Fix code36 request sent to ns #8964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b97f60f
[ISSUE #8963] Fix code36 request sent to ns
yx9o Nov 20, 2024
ef8deed
Update DefaultMQPullConsumerImpl.java
yx9o Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPullConsumerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,10 @@ public void sendMessageBack(MessageExt msg, int delayLevel, final String brokerN
String brokerAddr = (null != destBrokerName) ? this.mQClientFactory.findBrokerAddressInPublish(destBrokerName)
: RemotingHelper.parseSocketAddressAddr(msg.getStoreHost());

if (UtilAll.isBlank(brokerAddr)) {
throw new MQClientException("Broker[" + destBrokerName + "] master node does not exist", null);
}

if (UtilAll.isBlank(consumerGroup)) {
consumerGroup = this.defaultMQPullConsumer.getConsumerGroup();
}
Expand Down
3 changes: 3 additions & 0 deletions client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPushConsumerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,9 @@ private void sendMessageBack(MessageExt msg, int delayLevel, final String broker
} else {
String brokerAddr = (null != brokerName) ? this.mQClientFactory.findBrokerAddressInPublish(brokerName)
: RemotingHelper.parseSocketAddressAddr(msg.getStoreHost());
if (UtilAll.isBlank(brokerAddr)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this code have the same issue ?

rocketmq/client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPullConsumerImpl.java

Lines 649 to 651 in a8779c0

String brokerAddr = (null != destBrokerName) ? this.mQClientFactory.findBrokerAddressInPublish(destBrokerName)
: RemotingHelper.parseSocketAddressAddr(msg.getStoreHost());

Copy link
Contributor Author

@yx9o yx9o Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Logically, it is the same as push and will have this problem, but notice that it has been marked as @Deprecated. Does it need to be modified?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, maybe someone still using this, it would be better to modify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I will change it together.

throw new MQClientException("Broker[" + brokerName + "] master node does not exist", null);
}
this.mQClientFactory.getMQClientAPIImpl().consumerSendMessageBack(brokerAddr, brokerName, msg,
this.defaultMQPushConsumer.getConsumerGroup(), delayLevel, 5000, getMaxReconsumeTimes());
}
Expand Down
Loading