You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/04/17 09:08:43 UTC
[GitHub] [rocketmq] MatrixHB opened a new pull request, #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
MatrixHB opened a new pull request, #4179:
URL: https://github.com/apache/rocketmq/pull/4179
[#4154 ](https://github.com/apache/rocketmq/issues/4154)
--
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
[GitHub] [rocketmq] github-actions[bot] commented on pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#issuecomment-1705784062
This PR was closed because it has been inactive for 3 days since being marked as stale.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] mz0113 commented on a diff in pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by GitBox <gi...@apache.org>.
mz0113 commented on code in PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#discussion_r949280234
##########
client/src/main/java/org/apache/rocketmq/client/latency/MQFaultStrategy.java:
##########
@@ -69,16 +69,17 @@ public MessageQueue selectOneMessageQueue(final TopicPublishInfo tpInfo, final S
}
final String notBestBroker = latencyFaultTolerance.pickOneAtLeast();
- int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
- if (writeQueueNums > 0) {
- final MessageQueue mq = tpInfo.selectOneMessageQueue();
- if (notBestBroker != null) {
+ if (notBestBroker != null) {
+ int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
+ if (writeQueueNums > 0) {
+ MessageQueue mq = new MessageQueue();
+ mq.setTopic(tpInfo.getMessageQueueList().get(0).getTopic());
Review Comment:
> Why use Topic0 directly here?
回复错人了我晕，我说这么久还没合进去。这个是因为 tpInfo.getMessageQueueList() 的queue全部都是同一个topic下的，所以你取这个list的哪一个元素都行，直接取0是因为代码在这个位置可以明确这个list至少有一个元素在里面，所以取0不会导致数组越界，写其他的值可能导致越界。
73行 getQueueIdByBroker() 返回的writeQueueNums>0 就说明取到了queueId,既然取到了queueId那tpInfo.getMessageQueueList() 就不可能是空的list
--
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
[GitHub] [rocketmq] tsunghanjacktsai commented on a diff in pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by GitBox <gi...@apache.org>.
tsunghanjacktsai commented on code in PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#discussion_r891867686
##########
client/src/main/java/org/apache/rocketmq/client/latency/MQFaultStrategy.java:
##########
@@ -69,16 +69,17 @@ public MessageQueue selectOneMessageQueue(final TopicPublishInfo tpInfo, final S
}
final String notBestBroker = latencyFaultTolerance.pickOneAtLeast();
- int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
- if (writeQueueNums > 0) {
- final MessageQueue mq = tpInfo.selectOneMessageQueue();
- if (notBestBroker != null) {
+ if (notBestBroker != null) {
+ int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
+ if (writeQueueNums > 0) {
+ MessageQueue mq = new MessageQueue();
+ mq.setTopic(tpInfo.getMessageQueueList().get(0).getTopic());
Review Comment:
@MatrixHB This. Same confusion here. Could you please explain why directly apply topic 0 here?
--
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
[GitHub] [rocketmq] github-actions[bot] closed pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
URL: https://github.com/apache/rocketmq/pull/4179
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hzh0425 commented on a diff in pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on code in PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#discussion_r855692287
##########
client/src/main/java/org/apache/rocketmq/client/latency/MQFaultStrategy.java:
##########
@@ -69,16 +69,17 @@ public MessageQueue selectOneMessageQueue(final TopicPublishInfo tpInfo, final S
}
final String notBestBroker = latencyFaultTolerance.pickOneAtLeast();
- int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
- if (writeQueueNums > 0) {
- final MessageQueue mq = tpInfo.selectOneMessageQueue();
- if (notBestBroker != null) {
+ if (notBestBroker != null) {
+ int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
+ if (writeQueueNums > 0) {
+ MessageQueue mq = new MessageQueue();
+ mq.setTopic(tpInfo.getMessageQueueList().get(0).getTopic());
Review Comment:
Why use Topic0 directly here?
--
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
[GitHub] [rocketmq] github-actions[bot] commented on pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#issuecomment-1703536198
This PR is stale because it has been open for 365 days with no activity. It will be closed in 3 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] mz0113 commented on pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by GitBox <gi...@apache.org>.
mz0113 commented on PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#issuecomment-1219611437
哎，怎么还没合进去 还是open状态
--
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
[GitHub] [rocketmq] mz0113 commented on a diff in pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by GitBox <gi...@apache.org>.
mz0113 commented on code in PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#discussion_r913318999
##########
client/src/main/java/org/apache/rocketmq/client/latency/MQFaultStrategy.java:
##########
@@ -69,16 +69,17 @@ public MessageQueue selectOneMessageQueue(final TopicPublishInfo tpInfo, final S
}
final String notBestBroker = latencyFaultTolerance.pickOneAtLeast();
- int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
- if (writeQueueNums > 0) {
- final MessageQueue mq = tpInfo.selectOneMessageQueue();
- if (notBestBroker != null) {
+ if (notBestBroker != null) {
+ int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
+ if (writeQueueNums > 0) {
+ MessageQueue mq = new MessageQueue();
+ mq.setTopic(tpInfo.getMessageQueueList().get(0).getTopic());
Review Comment:
> @MatrixHB Second this. Same confusion here. Could you please explain why directly apply topic 0 here?
the topic of all messageQueue in the tpInfo.getMessageQueueList() is same,so you can choose any one,but if you get(1,2,3,4,....),may cause arrayIndexOutOfboundsException,so we get(0),if the size() ==0,the method will not be entered because no messageQueue can be used at begin
--
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
[GitHub] [rocketmq] mz0113 commented on a diff in pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by GitBox <gi...@apache.org>.
mz0113 commented on code in PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#discussion_r913318999
##########
client/src/main/java/org/apache/rocketmq/client/latency/MQFaultStrategy.java:
##########
@@ -69,16 +69,17 @@ public MessageQueue selectOneMessageQueue(final TopicPublishInfo tpInfo, final S
}
final String notBestBroker = latencyFaultTolerance.pickOneAtLeast();
- int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
- if (writeQueueNums > 0) {
- final MessageQueue mq = tpInfo.selectOneMessageQueue();
- if (notBestBroker != null) {
+ if (notBestBroker != null) {
+ int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
+ if (writeQueueNums > 0) {
+ MessageQueue mq = new MessageQueue();
+ mq.setTopic(tpInfo.getMessageQueueList().get(0).getTopic());
Review Comment:
> @MatrixHB Second this. Same confusion here. Could you please explain why directly apply topic 0 here?
the topic of all messageQueue in the tpInfo.getMessageQueueList() is same,so you can choose any one,but if you get(1,2,3,4,....),may cause arrayIndexOutOfboundsException,so we get(0),if the size() ==0,it would not go into this method
--
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
[GitHub] [rocketmq] coveralls commented on pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#issuecomment-1100840524
[](https://coveralls.io/builds/48332493)
Coverage increased (+0.2%) to 52.076% when pulling **0bfd2ff16f8a6a6eef61c7e0243f638f6b065bf5 on MatrixHB:huitong_fix_select_queue** into **aa908ce825e5e20bcc0d7b81c8c1e241a81664fe on apache:develop**.
--
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
[GitHub] [rocketmq] tsunghanjacktsai commented on a diff in pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by GitBox <gi...@apache.org>.
tsunghanjacktsai commented on code in PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#discussion_r891867686
##########
client/src/main/java/org/apache/rocketmq/client/latency/MQFaultStrategy.java:
##########
@@ -69,16 +69,17 @@ public MessageQueue selectOneMessageQueue(final TopicPublishInfo tpInfo, final S
}
final String notBestBroker = latencyFaultTolerance.pickOneAtLeast();
- int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
- if (writeQueueNums > 0) {
- final MessageQueue mq = tpInfo.selectOneMessageQueue();
- if (notBestBroker != null) {
+ if (notBestBroker != null) {
+ int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
+ if (writeQueueNums > 0) {
+ MessageQueue mq = new MessageQueue();
+ mq.setTopic(tpInfo.getMessageQueueList().get(0).getTopic());
Review Comment:
@MatrixHB Second this. Same confusion here. Could you please explain why directly apply topic 0 here?
--
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
[GitHub] [rocketmq] mz0113 commented on a diff in pull request #4179: [ISSUE #4154] fix the thread-safe problem in `selectOneMessageQueue`
Posted by GitBox <gi...@apache.org>.
mz0113 commented on code in PR #4179:
URL: https://github.com/apache/rocketmq/pull/4179#discussion_r949280234
##########
client/src/main/java/org/apache/rocketmq/client/latency/MQFaultStrategy.java:
##########
@@ -69,16 +69,17 @@ public MessageQueue selectOneMessageQueue(final TopicPublishInfo tpInfo, final S
}
final String notBestBroker = latencyFaultTolerance.pickOneAtLeast();
- int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
- if (writeQueueNums > 0) {
- final MessageQueue mq = tpInfo.selectOneMessageQueue();
- if (notBestBroker != null) {
+ if (notBestBroker != null) {
+ int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
+ if (writeQueueNums > 0) {
+ MessageQueue mq = new MessageQueue();
+ mq.setTopic(tpInfo.getMessageQueueList().get(0).getTopic());
Review Comment:
> Why use Topic0 directly here?
回复错人了我晕，我说这么久还没合进去。这个是因为 tpInfo.getMessageQueueList() 的queue全部都是同一个topic下的，所以你取这个list的哪一个元素都行，直接取0是因为代码在这个位置可以明确这个list至少有一个元素在里面，所以取0不会导致数组越界，写其他的值可能导致越界。
--
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