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/06/13 03:31:23 UTC

[GitHub] [rocketmq] dongeforever opened a new pull request, #4454: Do not chanage the RPC Interface to get flow control capability

dongeforever opened a new pull request, #4454:
URL: https://github.com/apache/rocketmq/pull/4454

**Make sure set the target branch to `develop`**

## What is the purpose of the change

Fix #4437 without changing the RPC hook interface.

## Brief changelog

## Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily. Notice, `it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR`.

- [x] Make sure there is a [Github issue](https://github.com/apache/rocketmq/issues) filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [x] Format the pull request title like `[ISSUE #123] Fix UnknownException when host config not exist`. Each commit in the pull request should have a meaningful subject line and body.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in [test module](https://github.com/apache/rocketmq/tree/master/test).
- [x] Run `mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle` to make sure basic checks pass. Run `mvn clean install -DskipITs` to make sure unit-test pass. Run `mvn clean test-compile failsafe:integration-test` to make sure integration-test pass.
- [ ] If this contribution is large, please file an [Apache Individual Contributor License Agreement](http://www.apache.org/licenses/#clas).

--
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] lizhanhui commented on pull request #4454: Do not chanage the RPC Interface to get flow control capability

Posted by GitBox <gi...@apache.org>.

lizhanhui commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1153518062

   Please discuss on the issue thread or the original PR before creating an 'alternative' one.


-- 
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 #4454: [Issue #4437]Possible alternative solution to get flow control capability without changing the RPCHook Interface

Posted by GitBox <gi...@apache.org>.

github-actions[bot] commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1262959444

   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: 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 #4454: [Issue #4437]Possible alternative solution to get flow control capability without changing the RPCHook Interface

Posted by GitBox <gi...@apache.org>.

coveralls commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1161645892

   
   [![Coverage Status](https://coveralls.io/builds/50207735/badge)](https://coveralls.io/builds/50207735)
   
   Coverage increased (+0.06%) to 52.077% when pulling **d6d9b820e03a39d1f4fe0048f2cdb4dac1627730 on dongeforever:no_change_rpchook_interface** into **32733e76747f3db761ace75cfed6d6fdcdaee81a 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] github-actions[bot] closed pull request #4454: [Issue #4437]Possible alternative solution to get flow control capability without changing the RPCHook Interface

Posted by GitBox <gi...@apache.org>.

github-actions[bot] closed pull request #4454: [Issue #4437]Possible alternative solution to get flow control capability without changing the RPCHook Interface
URL: https://github.com/apache/rocketmq/pull/4454


-- 
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] dongeforever commented on pull request #4454: Do not chanage the RPC Interface to get flow control capability

Posted by GitBox <gi...@apache.org>.

dongeforever commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1153515321

   @lizhanhui This PR needs a discussion before closing it.


-- 
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] lizhanhui commented on pull request #4454: [Issue #4437]Possible alternative solution to get flow control capability without changing the RPCHook Interface

Posted by GitBox <gi...@apache.org>.

lizhanhui commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1164186499

   > Since the original PR is closed,  now it is this PR's responsibility to solve the original issue
   
   I closed the original and much better solution because I do NOT see a chance where we can reach an agreement. As pointed out in the original pull request, this pull request suffers multiple drawbacks. I suggest NOT to merge it stealthily before an agreement is reached.


-- 
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] dongeforever commented on pull request #4454: Do not chanage the RPC Interface to get flow control capability

Posted by GitBox <gi...@apache.org>.

dongeforever commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1153523973

   @lizhanhui Yeah. it is intended to discuss it in the original PR. 
   
   In fact, this PR is based on the original PR and just a show.


-- 
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] lizhanhui closed pull request #4454: Do not chanage the RPC Interface to get flow control capability

Posted by GitBox <gi...@apache.org>.

lizhanhui closed pull request #4454: Do not chanage the RPC Interface to get flow control capability
URL: https://github.com/apache/rocketmq/pull/4454


-- 
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] lizhanhui commented on pull request #4454: Do not chanage the RPC Interface to get flow control capability

Posted by GitBox <gi...@apache.org>.

lizhanhui commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1153469406

   Duplicated


-- 
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] codecov-commenter commented on pull request #4454: [Issue #4437]Possible alternative solution to get flow control capability without changing the RPCHook Interface

Posted by GitBox <gi...@apache.org>.

codecov-commenter commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1161646219

   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4454?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 [#4454](https://codecov.io/gh/apache/rocketmq/pull/4454?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d6d9b82) into [develop](https://codecov.io/gh/apache/rocketmq/commit/32733e76747f3db761ace75cfed6d6fdcdaee81a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (32733e7) will **decrease** coverage by `0.09%`.
   > The diff coverage is `29.16%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #4454      +/-   ##
   =============================================
   - Coverage      48.16%   48.06%   -0.10%     
   + Complexity      5127     5125       -2     
   =============================================
     Files            649      652       +3     
     Lines          43021    43105      +84     
     Branches        5625     5642      +17     
   =============================================
   - Hits           20720    20718       -2     
   - Misses         19798    19872      +74     
   - Partials        2503     2515      +12     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4454?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../org/apache/rocketmq/remoting/AbstractRpcHook.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL0Fic3RyYWN0UnBjSG9vay5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...rocketmq/remoting/netty/NettyRemotingAbstract.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?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) | `45.14% <21.53%> (-5.96%)` | :arrow_down: |
   | [...e/rocketmq/remoting/netty/NettyRemotingClient.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `44.91% <29.16%> (-1.70%)` | :arrow_down: |
   | [...rocketmq/remoting/netty/DefaultRPCHookContext.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L0RlZmF1bHRSUENIb29rQ29udGV4dC5qYXZh) | `33.33% <33.33%> (ø)` | |
   | [...ocketmq/broker/processor/SendMessageProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL1NlbmRNZXNzYWdlUHJvY2Vzc29yLmphdmE=) | `40.15% <60.00%> (+0.10%)` | :arrow_up: |
   | [...he/rocketmq/test/client/rmq/RMQNormalProducer.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC9jbGllbnQvcm1xL1JNUU5vcm1hbFByb2R1Y2VyLmphdmE=) | `79.78% <75.00%> (ø)` | |
   | [...a/org/apache/rocketmq/remoting/RPCHookContext.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL1JQQ0hvb2tDb250ZXh0LmphdmE=) | `100.00% <100.00%> (ø)` | |
   | [...tmq/remoting/netty/AsyncNettyRequestProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L0FzeW5jTmV0dHlSZXF1ZXN0UHJvY2Vzc29yLmphdmE=) | `100.00% <100.00%> (ø)` | |
   | [...org/apache/rocketmq/common/stats/StatsItemSet.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvU3RhdHNJdGVtU2V0LmphdmE=) | `47.76% <0.00%> (-7.47%)` | :arrow_down: |
   | [...ache/rocketmq/common/stats/MomentStatsItemSet.java](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvTW9tZW50U3RhdHNJdGVtU2V0LmphdmE=) | `47.82% <0.00%> (-4.35%)` | :arrow_down: |
   | ... and [17 more](https://codecov.io/gh/apache/rocketmq/pull/4454/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/4454?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4454?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [32733e7...d6d9b82](https://codecov.io/gh/apache/rocketmq/pull/4454?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] dongeforever commented on pull request #4454: [Issue #4437]Possible alternative solution to get flow control capability without changing the RPCHook Interface

Posted by GitBox <gi...@apache.org>.

dongeforever commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1164035400

   @duhengforever @zhouxinyu  Since the original PR is closed, now it is this PR's responsibility to solve the original issue. 
   I have added some tests. please help a review.


-- 
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] dongeforever commented on pull request #4454: [Issue #4437]Possible alternative solution to get flow control capability without changing the RPCHook Interface

Posted by GitBox <gi...@apache.org>.

dongeforever commented on PR #4454:
URL: https://github.com/apache/rocketmq/pull/4454#issuecomment-1165343534

   @lizhanhui The original issue is valueable. And this solution causes none side effects and none API compatibility problems for the end-user. So it is free to polish it in the future PR.
   
   Anyway, wait for other member's reviews.


-- 
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