[GitHub] [rocketmq] lizhiboo commented on a diff in pull request #4562: UpdateTopicRouteInfoFromNameServer method in the concurrent lack of detection

[GitBox <gi...@apache.org>]

lizhiboo commented on code in PR #4562:
URL: https://github.com/apache/rocketmq/pull/4562#discussion_r914340669


##########
client/src/main/java/org/apache/rocketmq/client/impl/producer/DefaultMQProducerImpl.java:
##########
@@ -671,8 +676,18 @@ private SendResult sendDefaultImpl(
     private TopicPublishInfo tryToFindTopicPublishInfo(final String topic) {
         TopicPublishInfo topicPublishInfo = this.topicPublishInfoTable.get(topic);
         if (null == topicPublishInfo || !topicPublishInfo.ok()) {
-            this.topicPublishInfoTable.putIfAbsent(topic, new TopicPublishInfo());
-            this.mQClientFactory.updateTopicRouteInfoFromNameServer(topic);
+            try {
+                if(lockUpdatePublishInfo.tryLock(LOCK_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)){

Review Comment:
   IMO, add a new lock before lockNamesrv.tryLock do not make effect. We can check topic route info exists or not again after lockNamesrv.tryLock success to avoid redundant getRouteInfoByTopic.



-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org