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 2019/12/19 06:44:58 UTC
[GitHub] [rocketmq] rushsky518 opened a new pull request #1669: clean code
rushsky518 opened a new pull request #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669
org.apache.rocketmq.broker.client.ProducerManager#getAvaliableChannel
is too redundant
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] rushsky518 closed pull request #1669: clean code
Posted by GitBox <gi...@apache.org>.
rushsky518 closed pull request #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] coveralls edited a comment on issue #1669: clean code
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669#issuecomment-567365350
[](https://coveralls.io/builds/27808161)
Coverage decreased (-0.1%) to 50.534% when pulling **4f6fb7c54e6f3ade897741240bb9c1bdf564d6bd on rushsky518:develop** into **f8f6fbe4aa7f5dee937e688322628c366b12a552 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] rushsky518 opened a new pull request #1669: clean code
Posted by GitBox <gi...@apache.org>.
rushsky518 opened a new pull request #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669
org.apache.rocketmq.broker.client.ProducerManager#getAvaliableChannel
is too redundant
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] rushsky518 closed pull request #1669: clean code
Posted by GitBox <gi...@apache.org>.
rushsky518 closed pull request #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] coveralls edited a comment on issue #1669: clean code
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669#issuecomment-567365350
[](https://coveralls.io/builds/27808480)
Coverage decreased (-0.1%) to 50.528% when pulling **c51289bea79143b9dc0e770fdb42d2436f83cee1 on rushsky518:develop** into **f8f6fbe4aa7f5dee937e688322628c366b12a552 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] xiangwangcheng commented on a change in pull request
#1669: clean code
Posted by GitBox <gi...@apache.org>.
xiangwangcheng commented on a change in pull request #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669#discussion_r361560571
##########
File path: broker/src/main/java/org/apache/rocketmq/broker/client/ProducerManager.java
##########
@@ -206,35 +206,31 @@ public void unregisterProducer(final String group, final ClientChannelInfo clien
}
public Channel getAvaliableChannel(String groupId) {
- HashMap<Channel, ClientChannelInfo> channelClientChannelInfoHashMap = groupChannelTable.get(groupId);
- List<Channel> channelList = new ArrayList<Channel>();
- if (channelClientChannelInfoHashMap != null) {
- for (Channel channel : channelClientChannelInfoHashMap.keySet()) {
- channelList.add(channel);
- }
- int size = channelList.size();
- if (0 == size) {
- log.warn("Channel list is empty. groupId={}", groupId);
- return null;
- }
+ Channel checkChannel = null;
+ List<Channel> channelList = null;
- int index = positiveAtomicCounter.incrementAndGet() % size;
- Channel channel = channelList.get(index);
- int count = 0;
- boolean isOk = channel.isActive() && channel.isWritable();
- while (count++ < GET_AVALIABLE_CHANNEL_RETRY_COUNT) {
- if (isOk) {
- return channel;
- }
- index = (++index) % size;
- channel = channelList.get(index);
- isOk = channel.isActive() && channel.isWritable();
- }
- } else {
+ HashMap<Channel, ClientChannelInfo> channelClientChannelInfoHashMap = groupChannelTable.get(groupId);
+ if (null == channelClientChannelInfoHashMap || channelClientChannelInfoHashMap.isEmpty()) {
log.warn("Check transaction failed, channel table is empty. groupId={}", groupId);
- return null;
+ return checkChannel;
}
- return null;
+
+ channelList = new ArrayList<Channel>(channelClientChannelInfoHashMap.keySet());
+ int index = positiveAtomicCounter.incrementAndGet() % channelList.size();
+ Channel channel = null;
+ boolean isOk = false;
+ int count = 0;
+ do {
+ channel = channelList.get(index);
+ isOk = channel.isActive() && channel.isWritable();
+ if (isOk) {
+ checkChannel = channel;
Review comment:
`checkCannel` and `channel` local variable seem a little bit redundant. Is it cleaner to use 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] coveralls edited a comment on issue #1669: clean code
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669#issuecomment-567365350
[](https://coveralls.io/builds/27808626)
Coverage decreased (-0.04%) to 50.604% when pulling **a5e6df5056977e92cdaa1caa6927c5505517f64c on rushsky518:develop** into **f8f6fbe4aa7f5dee937e688322628c366b12a552 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] coveralls edited a comment on issue #1669: clean code
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669#issuecomment-567365350
[](https://coveralls.io/builds/27808467)
Coverage decreased (-0.05%) to 50.59% when pulling **c51289bea79143b9dc0e770fdb42d2436f83cee1 on rushsky518:develop** into **f8f6fbe4aa7f5dee937e688322628c366b12a552 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services