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 2020/12/16 02:08:09 UTC
[GitHub] [rocketmq] francisoliverlee opened a new pull request #2504: [Doc Error Fix] issue #2503
francisoliverlee opened a new pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504
## What is the purpose of the change
XXXXX
## Brief changelog
XX
## 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] francisoliverlee commented on pull request #2504: [doc error fix] issue #2503
Posted by GitBox <gi...@apache.org>.
francisoliverlee commented on pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504#issuecomment-749300165
> @francisoliverlee Thanks, I got your concerns. Generally speaking, a pr solves only one problem. I highly recommend that you could do that next time. Considering that the additional revision is only a spelling problem, I hope that we could set an example next time according to our rules:-)
-_-
----------------------------------------------------------------
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
[GitHub] [rocketmq] vongosling commented on a change in pull request #2504: [doc error fix] issue #2503
Posted by GitBox <gi...@apache.org>.
vongosling commented on a change in pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504#discussion_r546661563
##########
File path: tools/src/main/java/org/apache/rocketmq/tools/admin/DefaultMQAdminExtImpl.java
##########
@@ -1051,4 +1054,29 @@ public boolean resumeCheckHalfMessage(final String topic, final String msgId) th
return this.mqClientInstance.getMQClientAPIImpl().resumeCheckHalfMessage(RemotingUtil.socketAddress2String(msg.getStoreHost()), msgClient.getOffsetMsgId(), timeoutMillis);
}
}
+
+ /**
+ * check the consumer group instance use java or not
+ * for now, only one scenario can judged right: all instances in the group use the same language
+ * if >= 2 languages used in group instances, it will return false
+ *
+ * good news is for common ways, the coder would use one language for the group
+ * @param group consumer group name
+ * @return true: all is java instances, false: not sure
+ */
+ public boolean isAllInstancesInGroupJava(String group) throws InterruptedException, RemotingException, MQClientException, MQBrokerException {
+ ConsumerConnection connection = this.examineConsumerConnectionInfo(group);
+ if (connection.getConnectionSet().isEmpty()) {
+ return false;
+ }
+ boolean isJava = false;
+ for (Connection con : connection.getConnectionSet()) {
+ if (LanguageCode.JAVA == con.getLanguage()) {
+ //if >=2 languages used in instances, it can't be judged
Review comment:
@ShannonDing Would you like to help update some logic only works well for java?
----------------------------------------------------------------
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
[GitHub] [rocketmq] vongosling commented on a change in pull request #2504: [doc error fix] issue #2503
Posted by GitBox <gi...@apache.org>.
vongosling commented on a change in pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504#discussion_r548952449
##########
File path: tools/src/main/java/org/apache/rocketmq/tools/admin/DefaultMQAdminExtImpl.java
##########
@@ -1051,4 +1054,29 @@ public boolean resumeCheckHalfMessage(final String topic, final String msgId) th
return this.mqClientInstance.getMQClientAPIImpl().resumeCheckHalfMessage(RemotingUtil.socketAddress2String(msg.getStoreHost()), msgClient.getOffsetMsgId(), timeoutMillis);
}
}
+
+ /**
+ * check the consumer group instance use java or not
+ * for now, only one scenario can judged right: all instances in the group use the same language
+ * if >= 2 languages used in group instances, it will return false
+ *
+ * good news is for common ways, the coder would use one language for the group
+ * @param group consumer group name
+ * @return true: all is java instances, false: not sure
+ */
+ public boolean isAllInstancesInGroupJava(String group) throws InterruptedException, RemotingException, MQClientException, MQBrokerException {
+ ConsumerConnection connection = this.examineConsumerConnectionInfo(group);
+ if (connection.getConnectionSet().isEmpty()) {
+ return false;
+ }
+ boolean isJava = false;
+ for (Connection con : connection.getConnectionSet()) {
+ if (LanguageCode.JAVA == con.getLanguage()) {
+ //if >=2 languages used in instances, it can't be judged
Review comment:
@ShannonDing Ping...
----------------------------------------------------------------
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
[GitHub] [rocketmq] vongosling commented on pull request #2504: [doc error fix] issue #2503
Posted by GitBox <gi...@apache.org>.
vongosling commented on pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504#issuecomment-748934300
@francisoliverlee Thanks, I got your concerns. Generally speaking, a pr solves only one problem. I highly recommend that you could do that next time. Considering that the additional revision is only a spelling problem, I hope that we could set an example next time according to our rules:-)
----------------------------------------------------------------
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
[GitHub] [rocketmq] codecov-io commented on pull request #2504: [doc error fix] issue #2503
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504#issuecomment-749329729
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/2504?src=pr&el=h1) Report
> Merging [#2504](https://codecov.io/gh/apache/rocketmq/pull/2504?src=pr&el=desc) (f35cafe) into [develop](https://codecov.io/gh/apache/rocketmq/commit/9f95a972e10e0681bc3f2d00e9957aa212e897b5?el=desc) (9f95a97) will **increase** coverage by `0.43%`.
> The diff coverage is `66.19%`.
[](https://codecov.io/gh/apache/rocketmq/pull/2504?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #2504 +/- ##
=============================================
+ Coverage 45.69% 46.13% +0.43%
- Complexity 4271 4330 +59
=============================================
Files 546 547 +1
Lines 35860 36236 +376
Branches 4763 4808 +45
=============================================
+ Hits 16388 16718 +330
- Misses 17429 17436 +7
- Partials 2043 2082 +39
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/2504?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../java/org/apache/rocketmq/acl/common/AclUtils.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvY29tbW9uL0FjbFV0aWxzLmphdmE=) | `83.12% <ø> (ø)` | `66.00 <0.00> (ø)` | |
| [...ava/org/apache/rocketmq/acl/common/Permission.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvY29tbW9uL1Blcm1pc3Npb24uamF2YQ==) | `90.32% <ø> (-0.31%)` | `19.00 <0.00> (-1.00)` | |
| [...cketmq/broker/processor/ReplyMessageProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL1JlcGx5TWVzc2FnZVByb2Nlc3Nvci5qYXZh) | `54.91% <0.00%> (ø)` | `8.00 <0.00> (ø)` | |
| [...pache/rocketmq/client/common/ThreadLocalIndex.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29tbW9uL1RocmVhZExvY2FsSW5kZXguamF2YQ==) | `100.00% <ø> (+25.00%)` | `4.00 <0.00> (ø)` | |
| [...r/rebalance/AllocateMessageQueueByMachineRoom.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvcmViYWxhbmNlL0FsbG9jYXRlTWVzc2FnZVF1ZXVlQnlNYWNoaW5lUm9vbS5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...ent/impl/consumer/DefaultLitePullConsumerImpl.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9EZWZhdWx0TGl0ZVB1bGxDb25zdW1lckltcGwuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...lient/impl/consumer/DefaultMQPushConsumerImpl.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9EZWZhdWx0TVFQdXNoQ29uc3VtZXJJbXBsLmphdmE=) | `8.86% <ø> (+0.06%)` | `11.00 <0.00> (ø)` | |
| [...he/rocketmq/client/impl/consumer/ProcessQueue.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9Qcm9jZXNzUXVldWUuamF2YQ==) | `61.86% <ø> (-0.94%)` | `32.00 <0.00> (ø)` | |
| [.../rocketmq/client/impl/consumer/PullAPIWrapper.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9QdWxsQVBJV3JhcHBlci5qYXZh) | `51.28% <0.00%> (ø)` | `13.00 <0.00> (ø)` | |
| [...mq/client/impl/producer/DefaultMQProducerImpl.java](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9wcm9kdWNlci9EZWZhdWx0TVFQcm9kdWNlckltcGwuamF2YQ==) | `37.22% <0.00%> (+1.01%)` | `71.00 <0.00> (+2.00)` | |
| ... and [56 more](https://codecov.io/gh/apache/rocketmq/pull/2504/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/2504?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/2504?src=pr&el=footer). Last update [9f95a97...17107a7](https://codecov.io/gh/apache/rocketmq/pull/2504?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
[GitHub] [rocketmq] coveralls commented on pull request #2504: [doc error fix] issue #2503
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504#issuecomment-745721529
[](https://coveralls.io/builds/35747176)
Coverage increased (+0.1%) to 51.89% when pulling **17107a7d0db593eeb186ea117a23e58e40badfb4 on francisoliverlee:2020-12-16-issue2503-fix-doc-error** into **f35cafe1284547be52709ea7a14608d9fa90d48f 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
[GitHub] [rocketmq] dongeforever commented on pull request #2504: [doc error fix] issue #2503
Posted by GitBox <gi...@apache.org>.
dongeforever commented on pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504#issuecomment-745808058
@francisoliverlee I checked the change log，it does not only contain doc error fix, but also some code changes.
Please polish this pr's description, and make it more clear for reviewers.
----------------------------------------------------------------
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
[GitHub] [rocketmq] francisoliverlee commented on pull request #2504: [doc error fix] issue #2503
Posted by GitBox <gi...@apache.org>.
francisoliverlee commented on pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504#issuecomment-747161449
@dongeforever @vongosling
this PR expected to solve doc error only. but somwhow take other PR's(https://github.com/apache/rocketmq/pull/1930/files) changed-files with it. i would change the PR's description for review better.
----------------------------------------------------------------
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
[GitHub] [rocketmq] vongosling commented on pull request #2504: [doc error fix] issue #2503
Posted by GitBox <gi...@apache.org>.
vongosling commented on pull request #2504:
URL: https://github.com/apache/rocketmq/pull/2504#issuecomment-746072868
Why would our members submit a pr that does not follow the pr specification?
----------------------------------------------------------------
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