You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by Jaskey <gi...@git.apache.org> on 2017/02/15 14:19:46 UTC
[GitHub] incubator-rocketmq pull request #63: [ROCKETMQ-101]Fix possible NullPointerE...
GitHub user Jaskey opened a pull request:
https://github.com/apache/incubator-rocketmq/pull/63
[ROCKETMQ-101]Fix possible NullPointerException when retry in send Async way
JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-101
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Jaskey/incubator-rocketmq ROCKETMQ-101-NPE-when-retry
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-rocketmq/pull/63.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #63
----
commit 81baaf6a2165e2fb947aab56c1b03e7646f65b44
Author: Jaskey <li...@gmail.com>
Date: 2017-02-15T14:17:47Z
Fix possible NullPointerException when retry in send Async way
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
Merged
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
Looks good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
This issue is valid and the bug will be triggered under the following conditions:
1. A target message queue is specified, which will pass null for TopicPublishInfo parameter;
2. Async sending;
3. Retry happens;
4. sendLatencyFaultEnable is enabled(On default, it's not enabled)
The PR, IMO, is not good as it simply disabled promised retry attempts without notice. Instead of logging retry failure, retry sending should be done against the original chosen broker.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
@Jaskey I'll merge this PR soon. Please close this issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
[](https://coveralls.io/builds/10175883)
Coverage increased (+0.03%) to 31.549% when pulling **a8802d0b153e0f452d7147d0f70bc9cbf57e46de on Jaskey:ROCKETMQ-101-NPE-when-retry** into **573b22c37806a21543b90707bcce6022243a62da on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
@Jaskey You are right, I'll follow up and merge this PR ASAP.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
LGTM~
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
[](https://coveralls.io/builds/10175866)
Coverage increased (+0.09%) to 31.614% when pulling **a8802d0b153e0f452d7147d0f70bc9cbf57e46de on Jaskey:ROCKETMQ-101-NPE-when-retry** into **573b22c37806a21543b90707bcce6022243a62da on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
Thank you @vongosling @lizhanhui @zhouxinyu
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #63: [ROCKETMQ-101]Fix possible NullPointerE...
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey closed the pull request at:
https://github.com/apache/incubator-rocketmq/pull/63
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
@lizhanhui @Jaskey we'd better close after merge :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
Yes, #4 is not required as MQFaultStrategy#92 may also generate NPE.
Queue ID info is contained in the request header, minimal effort suffices.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
[](https://coveralls.io/builds/10190259)
Coverage increased (+0.02%) to 31.535% when pulling **18de2995e524135144159816dae75e17ebe9d9c1 on Jaskey:ROCKETMQ-101-NPE-when-retry** into **573b22c37806a21543b90707bcce6022243a62da on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #63: [ROCKETMQ-101]Fix possible NullPointerE...
Posted by Jaskey <gi...@git.apache.org>.
GitHub user Jaskey reopened a pull request:
https://github.com/apache/incubator-rocketmq/pull/63
[ROCKETMQ-101]Fix possible NullPointerException when retry in send Async way
JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-101
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Jaskey/incubator-rocketmq ROCKETMQ-101-NPE-when-retry
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-rocketmq/pull/63.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #63
----
commit 18de2995e524135144159816dae75e17ebe9d9c1
Author: Jaskey <li...@gmail.com>
Date: 2017-02-15T14:17:47Z
Fix possible NullPointerException when retry in send Async way
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
@lizhanhui , please review the updated pr
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
@lizhanhui , # 4 is not the nessary conditions, since even it is not enble, the tpInfo's method is still used.
try {
int index = tpInfo.getSendWhichQueue().getAndIncrement();
for (int i = 0; i < tpInfo.getMessageQueueList().size(); i++) {
int pos = Math.abs(index++) % tpInfo.getMessageQueueList().size();
if (pos < 0)
pos = 0;
MessageQueue mq = tpInfo.getMessageQueueList().get(pos);
if (latencyFaultTolerance.isAvailable(mq.getBrokerName())) {
if (null == lastBrokerName || mq.getBrokerName().equals(lastBrokerName))
return mq;
}
}
final String notBestBroker = latencyFaultTolerance.pickOneAtLeast();
int writeQueueNums = tpInfo.getQueueIdByBroker(notBestBroker);
if (writeQueueNums > 0) {
final MessageQueue mq = tpInfo.selectOneMessageQueue();
if (notBestBroker != null) {
mq.setBrokerName(notBestBroker);
mq.setQueueId(tpInfo.getSendWhichQueue().getAndIncrement() % writeQueueNums);
}
return mq;
} else {
latencyFaultTolerance.remove(notBestBroker);//my npe is thrown here
}
}
Besides, if topic route info is null which propably means user is send through my seletor method, resend should still respect user's seletor, so chosen broker is not enough, the chosen queue is needed, which may be another issue, I guess it is not a very minal effort, since the existing interface does not record any chosen queue info.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #63: [ROCKETMQ-101]Fix possible NullPointerE...
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey closed the pull request at:
https://github.com/apache/incubator-rocketmq/pull/63
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
[](https://coveralls.io/builds/10784736)
Coverage decreased (-0.5%) to 31.043% when pulling **18de2995e524135144159816dae75e17ebe9d9c1 on Jaskey:ROCKETMQ-101-NPE-when-retry** into **573b22c37806a21543b90707bcce6022243a62da on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...
Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/63
Looks good, please @vongosling help review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---