You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "haiyanghan (via GitHub)" <gi...@apache.org> on 2023/05/10 03:08:25 UTC
[GitHub] [rocketmq] haiyanghan opened a new pull request, #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
haiyanghan opened a new pull request, #6730:
URL: https://github.com/apache/rocketmq/pull/6730
[ISSUE#6726]
--
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] haiyanghan commented on a diff in pull request #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
Posted by "haiyanghan (via GitHub)" <gi...@apache.org>.
haiyanghan commented on code in PR #6730:
URL: https://github.com/apache/rocketmq/pull/6730#discussion_r1203487256
##########
broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java:
##########
@@ -1025,9 +1025,7 @@ public void registerProcessor() {
replyMessageProcessor.registerSendMessageHook(sendMessageHookList);
this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE, replyMessageProcessor, replyMessageExecutor);
- this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE_V2, replyMessageProcessor, replyMessageExecutor);
Review Comment:
there may compatibility issues, A better way is to add a new one SEND_REPLY_MESSAGE_V3 message code
--
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] haiyanghan commented on a diff in pull request #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
Posted by "haiyanghan (via GitHub)" <gi...@apache.org>.
haiyanghan commented on code in PR #6730:
URL: https://github.com/apache/rocketmq/pull/6730#discussion_r1203487256
##########
broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java:
##########
@@ -1025,9 +1025,7 @@ public void registerProcessor() {
replyMessageProcessor.registerSendMessageHook(sendMessageHookList);
this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE, replyMessageProcessor, replyMessageExecutor);
- this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE_V2, replyMessageProcessor, replyMessageExecutor);
Review Comment:
there may compatibility issues, A better way is to add a new one SEND_REPLY_MESSAGE_V3 message code and MessageCodeProcessor
--
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] haiyanghan commented on a diff in pull request #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
Posted by "haiyanghan (via GitHub)" <gi...@apache.org>.
haiyanghan commented on code in PR #6730:
URL: https://github.com/apache/rocketmq/pull/6730#discussion_r1211848325
##########
broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java:
##########
@@ -1025,9 +1025,7 @@ public void registerProcessor() {
replyMessageProcessor.registerSendMessageHook(sendMessageHookList);
this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE, replyMessageProcessor, replyMessageExecutor);
- this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE_V2, replyMessageProcessor, replyMessageExecutor);
Review Comment:
@aaron-ai hi, I have optimized the compatibility issue, Preserved `SEND_ REPLY_ MESSAGE` and `SEND_ REPLY_ MESSAGE_ V2` request code And the corresponding `processor`
--
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] codecov-commenter commented on pull request #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6730:
URL: https://github.com/apache/rocketmq/pull/6730#issuecomment-1541298681
## [Codecov](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6730](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a8d1eaa) into [develop](https://app.codecov.io/gh/apache/rocketmq/commit/f33ac2a3ece691bc15cb875726b5ad054a60ae22?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f33ac2a) will **decrease** coverage by `0.02%`.
> The diff coverage is `35.62%`.
```diff
@@ Coverage Diff @@
## develop #6730 +/- ##
=============================================
- Coverage 42.89% 42.87% -0.02%
- Complexity 9007 9016 +9
=============================================
Files 1113 1116 +3
Lines 78591 78469 -122
Branches 10221 10216 -5
=============================================
- Hits 33709 33643 -66
+ Misses 40659 40616 -43
+ Partials 4223 4210 -13
```
| [Impacted Files](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/rocketmq/broker/BrokerController.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvQnJva2VyQ29udHJvbGxlci5qYXZh) | `43.43% <ø> (-0.11%)` | :arrow_down: |
| [.../rocketmq/client/impl/ClientRemotingProcessor.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9DbGllbnRSZW1vdGluZ1Byb2Nlc3Nvci5qYXZh) | `3.25% <0.00%> (+0.37%)` | :arrow_up: |
| [...he/rocketmq/client/producer/DefaultMQProducer.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvcHJvZHVjZXIvRGVmYXVsdE1RUHJvZHVjZXIuamF2YQ==) | `60.88% <ø> (ø)` | |
| [.../java/org/apache/rocketmq/common/BrokerConfig.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vQnJva2VyQ29uZmlnLmphdmE=) | `27.99% <ø> (+0.01%)` | :arrow_up: |
| [...va/org/apache/rocketmq/common/message/Message.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vbWVzc2FnZS9NZXNzYWdlLmphdmE=) | `52.80% <0.00%> (-1.85%)` | :arrow_down: |
| [...g/apache/rocketmq/common/message/MessageReply.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vbWVzc2FnZS9NZXNzYWdlUmVwbHkuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...rocketmq/remoting/netty/NettyRemotingAbstract.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5UmVtb3RpbmdBYnN0cmFjdC5qYXZh) | `51.59% <0.00%> (-0.34%)` | :arrow_down: |
| [...apache/rocketmq/remoting/protocol/RequestCode.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL3Byb3RvY29sL1JlcXVlc3RDb2RlLmphdmE=) | `0.00% <ø> (ø)` | |
| [...ing/protocol/header/ReplyMessageRequestHeader.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL3Byb3RvY29sL2hlYWRlci9SZXBseU1lc3NhZ2VSZXF1ZXN0SGVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `22.88% <12.50%> (-0.31%)` | :arrow_down: |
| ... and [15 more](https://app.codecov.io/gh/apache/rocketmq/pull/6730?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
... and [16 files with indirect coverage changes](https://app.codecov.io/gh/apache/rocketmq/pull/6730/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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] aaron-ai commented on a diff in pull request #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
Posted by "aaron-ai (via GitHub)" <gi...@apache.org>.
aaron-ai commented on code in PR #6730:
URL: https://github.com/apache/rocketmq/pull/6730#discussion_r1203325804
##########
broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java:
##########
@@ -1025,9 +1025,7 @@ public void registerProcessor() {
replyMessageProcessor.registerSendMessageHook(sendMessageHookList);
this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE, replyMessageProcessor, replyMessageExecutor);
- this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE_V2, replyMessageProcessor, replyMessageExecutor);
Review Comment:
Why is the `SEND_REPLY_MESSAGE_V2` being removed? Will this cause any compatibility issues?
--
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] haiyanghan commented on a diff in pull request #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
Posted by "haiyanghan (via GitHub)" <gi...@apache.org>.
haiyanghan commented on code in PR #6730:
URL: https://github.com/apache/rocketmq/pull/6730#discussion_r1203487256
##########
broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java:
##########
@@ -1025,9 +1025,7 @@ public void registerProcessor() {
replyMessageProcessor.registerSendMessageHook(sendMessageHookList);
this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE, replyMessageProcessor, replyMessageExecutor);
- this.remotingServer.registerProcessor(RequestCode.SEND_REPLY_MESSAGE_V2, replyMessageProcessor, replyMessageExecutor);
Review Comment:
there may compatibility issues, A better way is to add a new one SEND_REPLY_MESSAGE_V3 request code and RequestCodeProcessor
--
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] haiyanghan commented on pull request #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
Posted by "haiyanghan (via GitHub)" <gi...@apache.org>.
haiyanghan commented on PR #6730:
URL: https://github.com/apache/rocketmq/pull/6730#issuecomment-1543269862
> Hey @haiyanghan , thanks for your effort at first. I probably see what you're trying to do here according to the issue you created, but the meaning of message sending in MQ just simply means that the Broker would return a corresponding response once the message arrived without waiting it to be consumed. Of course, you could detail your thought if I've misunderstood your intention for doing this.
> Hey @haiyanghan , thanks for your effort at first. I probably see what you're trying to do here according to the issue you created, but the meaning of message sending in MQ just simply means that the Broker would return a corresponding response once the message arrived without waiting it to be consumed. Of course, you could detail your thought if I've misunderstood your intention for doing this.
Sorry, my English proficiency is not very good and the problem description is not very clear
--
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] haiyanghan commented on pull request #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
Posted by "haiyanghan (via GitHub)" <gi...@apache.org>.
haiyanghan commented on PR #6730:
URL: https://github.com/apache/rocketmq/pull/6730#issuecomment-1560581716
@Guiqu1aixi hi, look 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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] haiyanghan commented on pull request #6730: [ISSUE apache#6726] Fix bug DefaultMQProducer#request method invoke t…
Posted by "haiyanghan (via GitHub)" <gi...@apache.org>.
haiyanghan commented on PR #6730:
URL: https://github.com/apache/rocketmq/pull/6730#issuecomment-1543269373
> Hey @haiyanghan , thanks for your effort at first. I probably see what you're trying to do here according to the issue you created, but the meaning of message sending in MQ just simply means that the Broker would return a corresponding response once the message arrived without waiting it to be consumed. Of course, you could detail your thought if I've misunderstood your intention for doing this.
Yes, when the message arrives at the broker, it returns, which is the logic of the original request method.
But the request method should be based on the request reply mode, waiting for the message to be consumed
--
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