You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "socutes (via GitHub)" <gi...@apache.org> on 2023/03/06 00:43:12 UTC
[GitHub] [rocketmq] socutes opened a new pull request, #6254: [ISSUE #6235] Some minor code optimizations
socutes opened a new pull request, #6254:
URL: https://github.com/apache/rocketmq/pull/6254
close #6235
--
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] socutes closed pull request #6254: [ISSUE #6235] Some minor code optimizations
Posted by "socutes (via GitHub)" <gi...@apache.org>.
socutes closed pull request #6254: [ISSUE #6235] Some minor code optimizations
URL: https://github.com/apache/rocketmq/pull/6254
--
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] caigy commented on a diff in pull request #6254: [ISSUE #6235] Some minor code optimizations
Posted by "caigy (via GitHub)" <gi...@apache.org>.
caigy commented on code in PR #6254:
URL: https://github.com/apache/rocketmq/pull/6254#discussion_r1125811381
##########
broker/src/main/java/org/apache/rocketmq/broker/processor/AckMessageProcessor.java:
##########
@@ -94,16 +94,15 @@ public boolean isPopReviveServiceRunning() {
@Override
public RemotingCommand processRequest(final ChannelHandlerContext ctx,
RemotingCommand request) throws RemotingCommandException {
- return this.processRequest(ctx.channel(), request, true);
+ return this.processRequest(ctx.channel(), request);
}
@Override
public boolean rejectRequest() {
return false;
}
- private RemotingCommand processRequest(final Channel channel, RemotingCommand request,
- boolean brokerAllowSuspend) throws RemotingCommandException {
+ private RemotingCommand processRequest(final Channel channel, RemotingCommand request) throws RemotingCommandException {
Review Comment:
This param may be useful in the future, so pls communicate with the community before modify it.
##########
broker/src/main/java/org/apache/rocketmq/broker/processor/AckMessageProcessor.java:
##########
@@ -160,8 +159,7 @@ private RemotingCommand processRequest(final Channel channel, RemotingCommand re
if (requestHeader.getOffset() < oldOffset) {
return response;
}
- while (!this.brokerController.getPopMessageProcessor().getQueueLockManager().tryLock(lockKey)) {
- }
+
Review Comment:
Could you explain the reason to remove the locking statement?
##########
broker/src/main/java/org/apache/rocketmq/broker/processor/ForwardRequestProcessor.java:
##########
@@ -1,45 +0,0 @@
-/*
Review Comment:
Have you launched a discuss about removing this class in community?
--
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 #6254: [ISSUE #6235] Some minor code optimizations
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6254:
URL: https://github.com/apache/rocketmq/pull/6254#issuecomment-1455280108
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/6254?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 [#6254](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9406582) into [develop](https://codecov.io/gh/apache/rocketmq/commit/18fcce664c8cf874f0da83a33976e09326021ee0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (18fcce6) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## develop #6254 +/- ##
==========================================
Coverage 43.20% 43.21%
- Complexity 8853 8856 +3
==========================================
Files 1094 1093 -1
Lines 77194 77186 -8
Branches 10070 10068 -2
==========================================
+ Hits 33349 33353 +4
+ Misses 39678 39666 -12
Partials 4167 4167
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...rocketmq/broker/processor/AckMessageProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0Fja01lc3NhZ2VQcm9jZXNzb3IuamF2YQ==) | `46.40% <100.00%> (+0.36%)` | :arrow_up: |
| [...broker/processor/ChangeInvisibleTimeProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0NoYW5nZUludmlzaWJsZVRpbWVQcm9jZXNzb3IuamF2YQ==) | `57.25% <100.00%> (+0.43%)` | :arrow_up: |
| [...mq/store/ha/autoswitch/AutoSwitchHAConnection.java](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL2F1dG9zd2l0Y2gvQXV0b1N3aXRjaEhBQ29ubmVjdGlvbi5qYXZh) | `70.16% <0.00%> (-1.62%)` | :arrow_down: |
| [...apache/rocketmq/store/ha/GroupTransferService.java](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL0dyb3VwVHJhbnNmZXJTZXJ2aWNlLmphdmE=) | `92.30% <0.00%> (-1.29%)` | :arrow_down: |
| [.../org/apache/rocketmq/store/ha/DefaultHAClient.java](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL0RlZmF1bHRIQUNsaWVudC5qYXZh) | `66.83% <0.00%> (-1.03%)` | :arrow_down: |
| [...cketmq/tieredstore/provider/TieredFileSegment.java](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dGllcmVkc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3RpZXJlZHN0b3JlL3Byb3ZpZGVyL1RpZXJlZEZpbGVTZWdtZW50LmphdmE=) | `73.88% <0.00%> (-0.75%)` | :arrow_down: |
| [...ent/impl/consumer/DefaultLitePullConsumerImpl.java](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9EZWZhdWx0TGl0ZVB1bGxDb25zdW1lckltcGwuamF2YQ==) | `67.53% <0.00%> (-0.47%)` | :arrow_down: |
| [...e/rocketmq/remoting/netty/NettyRemotingClient.java](https://codecov.io/gh/apache/rocketmq/pull/6254?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==) | `40.63% <0.00%> (-0.38%)` | :arrow_down: |
| [...cketmq/store/ha/autoswitch/AutoSwitchHAClient.java](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL2F1dG9zd2l0Y2gvQXV0b1N3aXRjaEhBQ2xpZW50LmphdmE=) | `76.07% <0.00%> (-0.36%)` | :arrow_down: |
| [...ava/org/apache/rocketmq/filter/util/BitsArray.java](https://codecov.io/gh/apache/rocketmq/pull/6254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmlsdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9maWx0ZXIvdXRpbC9CaXRzQXJyYXkuamF2YQ==) | `59.82% <0.00%> (ø)` | |
| ... and [8 more](https://codecov.io/gh/apache/rocketmq/pull/6254?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] socutes commented on pull request #6254: [ISSUE #6235] Some minor code optimizations
Posted by "socutes (via GitHub)" <gi...@apache.org>.
socutes commented on PR #6254:
URL: https://github.com/apache/rocketmq/pull/6254#issuecomment-1455710965
@caigy cai Sorry, I did not consider this requirement well, there is a problem with this PR, I will close 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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org