[GitHub] incubator-rocketmq issue #1: [ROCKETMQ-2] Closing the channel

2016-12-23 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/1 It seems ok, Thanks @shroman. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] incubator-rocketmq issue #5: [ROCKETMQ-8] Standardize build script.

2016-12-23 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/5 It seems ok. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes

[GitHub] incubator-rocketmq issue #7: [ROCKETMQ-3] Clean up and perfect the unit test...

2016-12-23 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/7 Good job! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] incubator-rocketmq issue #8: simplify if grammar

2016-12-23 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/8 Hi @naughtybear, Thanks your PR, could you abide our PR process(modify your PR subject and mention your jira address in the description), please follow this PR. #5 --- If your

[GitHub] incubator-rocketmq issue #13: [ROCKETMQ-15] Add config file. easy to change ...

2016-12-26 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/13 Hi @nottyjay There is no need to add `port` to NamesrvConfig, please check `NettyServerConfig.listenPort`. And I replied you that all the fields in *Config.java are

[GitHub] incubator-rocketmq issue #13: [ROCKETMQ-15] Add config file. easy to change ...

2016-12-26 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/13 Hi, Just config `listenPort` in `broker.conf` or `namesrv.conf`, the server will parse and use it. --- If your project is set up for it, you can reply to this email and have

[GitHub] incubator-rocketmq issue #15: [ROCKETMQ-16] Improve the codes of setting top...

2016-12-26 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/15 It looks good, please @vongosling @lollipopjin review it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] incubator-rocketmq issue #2: [ROCKETMQ-14]invoke callback shoule be invoked ...

2016-12-27 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/2 Hi, This PR has been merged, but next time please use JIRA issue name(`ROCKETMQ-14`) as branch name instead of `master`. Please refer to [here](http://rocketmq.apache.org/docs

[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 Hi, shroman Could you please resolve conflicts in this pr? And we will remain cautious about the changes of rocketmq-store module, so this pr may be won't merge qu

[GitHub] incubator-rocketmq pull request #20: [ROCKETMQ-23] MappedFileQueue#flush sho...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/20#discussion_r94042046 --- Diff: store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java --- @@ -39,11 +39,15 @@ private final int

[GitHub] incubator-rocketmq pull request #20: [ROCKETMQ-23] MappedFileQueue#flush sho...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/20#discussion_r94042593 --- Diff: store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java --- @@ -191,7 +199,7 @@ public long howMuchFallBehind

[GitHub] incubator-rocketmq pull request #20: [ROCKETMQ-23] MappedFileQueue#flush sho...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/20#discussion_r94043135 --- Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java --- @@ -964,9 +964,11 @@ public void run() { boolean

[GitHub] incubator-rocketmq issue #20: [ROCKETMQ-23] MappedFileQueue#flush should ret...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/20 please @vongosling @lollipopjin review this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not

[GitHub] incubator-rocketmq pull request #20: [ROCKETMQ-23] MappedFileQueue#flush sho...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/20#discussion_r94045317 --- Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java --- @@ -964,9 +964,11 @@ public void run() { boolean

[GitHub] incubator-rocketmq pull request #20: [ROCKETMQ-23] MappedFileQueue#flush sho...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/20#discussion_r94046163 --- Diff: store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java --- @@ -39,11 +39,15 @@ private final int

[GitHub] incubator-rocketmq issue #21: [ROCKETMQ-18]Clean code prompted by IntelliJ

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/21 It seems ok for me. @lollipopjin please review this pr. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

[GitHub] incubator-rocketmq pull request #21: [ROCKETMQ-18]Clean code prompted by Int...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94101345 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java --- @@ -469,6 +467,7 @@ private void onExceptionImpl(final

[GitHub] incubator-rocketmq pull request #21: [ROCKETMQ-18]Clean code prompted by Int...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94101409 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/factory/MQClientInstance.java --- @@ -1134,10 +1131,10 @@ public

[GitHub] incubator-rocketmq pull request #21: [ROCKETMQ-18]Clean code prompted by Int...

2016-12-28 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/21#discussion_r94101719 --- Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java --- @@ -1150,15 +1150,15 @@ public AppendMessageResult doAppend(final long

[GitHub] incubator-rocketmq-site issue #1: Initial best practise for broker,produer a...

2016-12-29 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq-site/pull/1 Good job. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and

[GitHub] incubator-rocketmq pull request #26: [ROCKETMQ-26]add new way to send messag...

2017-01-01 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/26#discussion_r94281902 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/producer/DefaultSendPromise.java --- @@ -0,0 +1,302 @@ +/* + * Licensed

[GitHub] incubator-rocketmq issue #23: [ROCKETMQ-5] Avoid creating directories in Uti...

2017-01-02 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/23 Great. Thanks @shroman --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] incubator-rocketmq issue #27: [ROCKETMQ-30] Fixed method signature for Messa...

2017-01-05 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/27 Cool, thanks @shroman --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] incubator-rocketmq-site issue #3: Split 'Git setup' for contributors and com...

2017-01-05 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq-site/pull/3 Thanks, @shroman --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled

[GitHub] incubator-rocketmq issue #29: [ROCKETMQ-31] Fix "No such file or directory" ...

2017-01-05 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/29 It's our mistake. As @iskl says, we print gc logs to tmpfs in order to eliminate long time latency brought by disk I/O competition. IMO, we can merge this PR at current

[GitHub] incubator-rocketmq pull request #31: [ROCKETMQ-33] CPU Occupy 100%

2017-01-07 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/31#discussion_r95059198 --- Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java --- @@ -1045,7 +1045,7 @@ public void run() { while

[GitHub] incubator-rocketmq issue #32: [ROCKETMQ-35] Consumer client can’t persist ...

2017-01-07 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/32 It's a bug indeed, thanks @qinliujie . Please @vongosling @lizhanhui review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitH

[GitHub] incubator-rocketmq pull request #31: [ROCKETMQ-33] CPU Occupy 100%

2017-01-07 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/31#discussion_r95059881 --- Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java --- @@ -1045,7 +1045,7 @@ public void run() { while

[GitHub] incubator-rocketmq issue #31: [ROCKETMQ-33] CPU Occupy 100%

2017-01-08 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/31 Thanks @qinliujie, please @vongosling @stevenschew help review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] incubator-rocketmq issue #30: [ROCKETMQ-34] Potential NPE in NettyConnetMana...

2017-01-08 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/30 Easy fix indeed, thanks @shroman , please @vongosling @stevenschew review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] incubator-rocketmq issue #29: [ROCKETMQ-31] Fix "No such file or directory" ...

2017-01-08 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/29 @shroman it's ok surely. Please @lizhanhui @stevenschew help review this pr. --- If your project is set up for it, you can reply to this email and have your reply appear on G

[GitHub] incubator-rocketmq issue #31: [ROCKETMQ-33] CPU Occupy 100%

2017-01-08 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/31 Hi, @qinliujie This PR will be merged soon, but next time please send PR from branch ROCKETMQ-33 instead of master. --- If your project is set up for it, you can reply to this email

[GitHub] incubator-rocketmq issue #32: [ROCKETMQ-35] Consumer client can’t persist ...

2017-01-08 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/32 @qinliujie , I can't find your Github email from https://github.com/qinliujie, could you please provide it for me which is need in merge process. --- If your project is set up f

[GitHub] incubator-rocketmq issue #3: [ROCKETMQ-6] Use logger for exceptions instead ...

2017-01-09 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/3 Hi, @shroman About `e.printStackTrace()`, the error message should log to file surely, but can we keep the `e.printStackTrace()` if the call stack is in broker start process

[GitHub] incubator-rocketmq issue #37: [ROCKETMQ-38] Some unit tests for rocketmq-rem...

2017-01-11 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/37 Hi @iskl, please make sure the PR can pass the CI test. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

[GitHub] incubator-rocketmq-site pull request #4: Improved 'How to create a PR (contr...

2017-01-12 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq-site/pull/4#discussion_r95922741 --- Diff: _docs/06-best-practice-pull-request.md --- @@ -73,11 +73,15 @@ Push your branch to Github: 6. When you are satisfied and want

[GitHub] incubator-rocketmq issue #41: 网络环境复杂时,会�� 成选择了不�...

2017-01-12 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/41 IMO, this feature is not necessary, clientIP is only used in bornHost. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] incubator-rocketmq issue #40: [ROCKETMQ-40]Ack mode support for consume conc...

2017-01-12 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/40 Hi, please resolve the conflicts and follow the rocketmq [codestyle](), and IMO, we need more discuss if we intent to add new feature to rocketmq-client module. --- If your project

[GitHub] incubator-rocketmq issue #39: [ROCKETMQ-45]Delete hanged consume queue files

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/39 Hi, @lizhanhui , what's the appearance/effect of this BUG? Let's start from the next round. IMO, it's ok when ConsumeQueue-MappedFile hold failed, th

[GitHub] incubator-rocketmq pull request #39: [ROCKETMQ-45]Delete hanged consume queu...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/39#discussion_r95955059 --- Diff: store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java --- @@ -397,11 +397,21 @@ public int deleteExpiredFileByOffset(long

[GitHub] incubator-rocketmq issue #41: 网络环境复杂时,会�� 成选择了不�...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/41 Hi @tain198127, could you please use English in the pr title and commit logs? Also please refer to [here](http://rocketmq.apache.org/docs/code-guidelines/) and [here](http

[GitHub] incubator-rocketmq issue #38: [ROCKETMQ-44] Refactor to avoid duplicated cod...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/38 Hi @wu-sheng, thanks for you PR, could please use the rocketmq code-style to reformat these commits? As for ```java } catch (Exception e) { } ``` or

[GitHub] incubator-rocketmq pull request #36: [ROCKETMQ-43] Remove NoWhitespaceBefore...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/36#discussion_r95956604 --- Diff: tools/src/main/java/org/apache/rocketmq/tools/command/MQAdminStartup.java --- @@ -194,7 +197,7 @@ public static void initCommand

[GitHub] incubator-rocketmq issue #33: [ROCKETMQ-37] Log output information is not ac...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/33 It seems ok, thanks @a2888409 Please @vongosling @lizhanhui help review this PR. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] incubator-rocketmq issue #28: [ROCKETMQ-27]Add store commitlog path online

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/28 Hi, @lizhanhui , Let's handle this PR in RocketMQ-4.1.0 as a new feature. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] incubator-rocketmq issue #8: [ROCKETMQ-32] Improve concision - Reuse local v...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/8 It seems ok, thanks @naughtybear . Please @vongosling @lizhanhui help review it. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] incubator-rocketmq issue #38: [ROCKETMQ-44] Refactor to avoid duplicated cod...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/38 @wu-sheng , sorry for my fault, I made some changes in intelliJ graph config page which doesn't reflect to the style file, anyway, I have updated the code style file, please che

[GitHub] incubator-rocketmq pull request #39: [ROCKETMQ-45]Delete hanged consume queu...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/39#discussion_r95964644 --- Diff: store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java --- @@ -397,11 +397,21 @@ public int deleteExpiredFileByOffset(long

[GitHub] incubator-rocketmq issue #39: [ROCKETMQ-45]Delete hanged consume queue files

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/39 I checked ConsumeQueue#correctMinOffset, and you are right this method won't work as expected in this situation. Please @vintagewang @vongosling help review this PR. -

[GitHub] incubator-rocketmq pull request #39: [ROCKETMQ-45]Delete hanged consume queu...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/39#discussion_r95979009 --- Diff: store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java --- @@ -397,11 +397,21 @@ public int deleteExpiredFileByOffset(long

[GitHub] incubator-rocketmq issue #39: [ROCKETMQ-45]Delete hanged consume queue files

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/39 @lizhanhui Also please add some unit tests -:).. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not

[GitHub] incubator-rocketmq pull request #37: [ROCKETMQ-38] Some unit tests for rocke...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/37#discussion_r96000424 --- Diff: remoting/src/test/java/org/apache/rocketmq/remoting/protocol/RemotingCommandTest.java --- @@ -0,0 +1,222 @@ +/* + * Licensed

[GitHub] incubator-rocketmq issue #37: [ROCKETMQ-38] Some unit tests for rocketmq-rem...

2017-01-13 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/37 It seems ok, please @vongosling @lizhanhui help review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

[GitHub] incubator-rocketmq-site issue #4: Improved 'How to create a PR (contributors...

2017-01-16 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq-site/pull/4 Sure, thanks @shroman --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] incubator-rocketmq issue #43: [ROCKETMQ-59] Change Charset usages in RocketM...

2017-01-16 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/43 @iskl It could be better if we narrow down `CHARSET_UTF8 ` access modifier to `private` and without `static`, for both `RocketMQSerializable` and `RemotingSerializable`. --- If your

[GitHub] incubator-rocketmq issue #34: [ROCKETMQ-39] avoid duplicated codes.

2017-01-19 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/34 It could be better if we add some unit tests for this PR and please remove the author info. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] incubator-rocketmq issue #38: [ROCKETMQ-44] Refactor to avoid duplicated cod...

2017-01-19 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/38 Hi @wu-sheng ,Could you please add some unit tests for this PR ? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] incubator-rocketmq pull request #46: [RocketMQ-58] Add integration test for ...

2017-01-19 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/46#discussion_r96842859 --- Diff: test/src/test/java/org/apache/rocketmq/test/client/producer/querymsg/QueryMsgByTopicIT.java --- @@ -0,0 +1,24

[GitHub] incubator-rocketmq pull request #46: [RocketMQ-58] Add integration test for ...

2017-01-19 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/46#discussion_r96841726 --- Diff: test/src/main/java/org/apache/rocketmq/test/util/MetaTestUtils.java --- @@ -0,0 +1,144 @@ +/* + * Licensed to the Apache

[GitHub] incubator-rocketmq issue #46: [RocketMQ-58] Add integration test for RocketM...

2017-01-19 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/46 Also thanks @fenglianghfl for this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this

[GitHub] incubator-rocketmq pull request #46: [RocketMQ-58] Add integration test for ...

2017-01-19 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/46#discussion_r96841483 --- Diff: pom.xml --- @@ -310,6 +290,7 @@ UTF-8 true

[GitHub] incubator-rocketmq pull request #46: [RocketMQ-58] Add integration test for ...

2017-01-19 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/46#discussion_r96842822 --- Diff: test/src/test/java/org/apache/rocketmq/test/client/producer/querymsg/QueryMsgByTopicExceptionIT.java --- @@ -0,0 +1,24

[GitHub] incubator-rocketmq issue #35: [ROCKETMQ-42] Output is not friendly, when Mai...

2017-01-19 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/35 Hi @wu-sheng , I am sorry for this rash close~ Actually, as @shroman mentioned in this [issue](https://github.com/apache/incubator-rocketmq/pull/3), we also think avoid using

[GitHub] incubator-rocketmq pull request #50: [ROCKETMQ-74] Fix DataVersion equals de...

2017-01-24 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/50#discussion_r97704748 --- Diff: common/src/main/java/org/apache/rocketmq/common/DataVersion.java --- @@ -58,16 +58,28 @@ public boolean equals(final Object o

[GitHub] incubator-rocketmq issue #50: [ROCKETMQ-74] Fix DataVersion equals defect

2017-01-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/50 Please @vongosling @lollipopjin help review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not

[GitHub] incubator-rocketmq issue #51: [ROCKETMQ-75] RemotingCommand header decoding ...

2017-01-31 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/51 Hi @shroman , IMO, it could be better that one PR/issue one thing, can we move `makeCustomHeaderToNet` to another issue, and make `makeCustomHeaderToNet ()` private will bring

[GitHub] incubator-rocketmq issue #53: [ROCKETMQ-80] Add batch feature

2017-02-07 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/53 I wonder that are there some compatibility issues between new client version and old server version? --- If your project is set up for it, you can reply to this email and have your

[GitHub] incubator-rocketmq issue #54: [ROCKETMQ-83] Fix quick start, annotate setNam...

2017-02-07 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/54 Hi, `quickStart.Consumer` also has the same issue, could you please fix it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] incubator-rocketmq issue #54: [ROCKETMQ-83] Fix quick start, annotate setNam...

2017-02-07 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/54 @vongosling I have created a new JIRA issue for it: https://issues.apache.org/jira/browse/ROCKETMQ-84 --- If your project is set up for it, you can reply to this email and have your

[GitHub] incubator-rocketmq issue #59: Add test case for LocalFileOffsetStore

2017-02-12 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/59 Hi, @djKooks We would appreciate your any contribution, please feel free to let us know if you have any question. Some docs may be for your reference: [1]. http

[GitHub] incubator-rocketmq issue #58: Update runbroker.sh

2017-02-12 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/58 Hi, @yuweitaocn Thanks for your PR, Do you have some test data to support your theory? The current GC parameters are well-tuned in our production env. --- If your project is set

[GitHub] incubator-rocketmq pull request #61: Fix risk of unable to release putMessag...

2017-02-14 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/61#discussion_r101196649 --- Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java --- @@ -799,27 +796,18 @@ public long lockTimeMills

[GitHub] incubator-rocketmq issue #66: [ROCKETMQ-106] Add flow control on topic level

2017-02-17 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/66 Great point, could you please add some unit tests? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] incubator-rocketmq issue #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...

2017-02-17 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/65 Hi @shroman , LGTM, but why do we add so many `@Ignore` annotations in this PR? All the he unit tests of tools module have been skipped in `tools/pom.xml`. We may forget to remove

[GitHub] incubator-rocketmq pull request #64: [ROCKETMQ-102] When shutdown(), the per...

2017-02-17 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/64#discussion_r101718137 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/ConsumeMessageConcurrentlyService.java --- @@ -93,8 +93,24 @@ public

[GitHub] incubator-rocketmq pull request #64: [ROCKETMQ-102] When shutdown(), the per...

2017-02-17 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/64#discussion_r101719487 --- Diff: client/src/main/java/org/apache/rocketmq/client/consumer/DefaultMQPushConsumer.java --- @@ -461,7 +461,11 @@ public void start

[GitHub] incubator-rocketmq issue #64: [ROCKETMQ-102] When shutdown(), the persisted ...

2017-02-17 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/64 Hi @Jaskey , Thanks for this PR, I added two comments for your reference. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] incubator-rocketmq pull request #67: [ROCKETMQ-67] Consistent Hash allocate ...

2017-02-20 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/67#discussion_r102124935 --- Diff: common/src/main/java/org/apache/rocketmq/common/consistenthash/Node.java --- @@ -0,0 +1,24 @@ +/* + * Licensed to the Apache

[GitHub] incubator-rocketmq pull request #64: [ROCKETMQ-102] When shutdown(), the per...

2017-02-20 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/64#discussion_r102128193 --- Diff: client/src/main/java/org/apache/rocketmq/client/consumer/DefaultMQPushConsumer.java --- @@ -461,7 +461,11 @@ public void start

[GitHub] incubator-rocketmq issue #68: [ROCKETMQ-107] fix possible concurrency proble...

2017-02-23 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/68 IMO, consider that `start/shutdown` is not a high frequency usage method, use `synchronized` may be more graceful and readable. please @lizhanhui @shroman help review at your

[GitHub] incubator-rocketmq issue #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...

2017-02-23 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/65 please @vongosling @lizhanhui help review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] incubator-rocketmq pull request #66: [ROCKETMQ-106] Add flow control on topi...

2017-02-23 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/66#discussion_r102890915 --- Diff: client/src/main/java/org/apache/rocketmq/client/consumer/DefaultMQPushConsumer.java --- @@ -166,11 +166,16 @@ private int

[GitHub] incubator-rocketmq issue #66: [ROCKETMQ-106] Add flow control on topic level

2017-02-23 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/66 LGTM, but please add some unit tests at your convenience. Please @vongosling @lizhanhui help review. --- If your project is set up for it, you can reply to this email and have your

[GitHub] incubator-rocketmq pull request #64: [ROCKETMQ-102] When shutdown(), the per...

2017-02-23 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/64#discussion_r102893082 --- Diff: client/src/test/java/org/apache/rocketmq/client/consumer/DefaultMQPushConsumerTest.java --- @@ -52,6 +56,7 @@ import

[GitHub] incubator-rocketmq pull request #64: [ROCKETMQ-102] When shutdown(), the per...

2017-02-23 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/64#discussion_r102893550 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/ConsumeMessageOrderlyService.java --- @@ -92,10 +92,22 @@ public void

[GitHub] incubator-rocketmq issue #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/63 Looks good, please @vongosling help review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] incubator-rocketmq issue #61: [ROCKETMQ-98]Fix risk of unable to release put...

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/61 Hi, @lizhanhui @shroman do you have some review results about this PR? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] incubator-rocketmq pull request #60: [ROCKETMQ-96]Rename some temp variable ...

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/60#discussion_r102896928 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/ProcessQueue.java --- @@ -46,7 +46,7 @@ private final TreeMap

[GitHub] incubator-rocketmq issue #59: Add test case for LocalFileOffsetStore

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/59 LGTM, please @vongosling @lizhanhui help review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not

[GitHub] incubator-rocketmq issue #58: Update runbroker.sh

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/58 @yuweitaocn , so, it seems that we could close this PR? what's your opinion? --- If your project is set up for it, you can reply to this email and have your reply appear on GitH

[GitHub] incubator-rocketmq pull request #57: [ROCKETMQ-91] Reduce lock granularity f...

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/57#discussion_r102897426 --- Diff: store/src/main/java/org/apache/rocketmq/store/CommitLog.java --- @@ -567,6 +577,11 @@ public PutMessageResult putMessage(final

[GitHub] incubator-rocketmq pull request #55: [ROCKETMQ-89] WS_DOMAIN_NAME, SUBGROUP ...

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/55#discussion_r102898060 --- Diff: broker/pom.xml --- @@ -66,4 +66,17 @@ javassist

[GitHub] incubator-rocketmq issue #52: [ROCKETMQ-76] Expose IntegrationTestBase to be...

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/52 Please @dongeforever help review this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] incubator-rocketmq issue #51: [ROCKETMQ-75] RemotingCommand header decoding ...

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/51 LGTM, thanks @shroman , please @vongosling @lizhanhui help review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] incubator-rocketmq issue #53: [ROCKETMQ-80] Add batch feature

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/53 Hi, @shroman @Jaskey @lizhanhui , what's your opinion about this updated PR? --- If your project is set up for it, you can reply to this email and have your reply appear on GitH

[GitHub] incubator-rocketmq issue #49: [ROCKETMQ-72] DefaultMessageStore cannot be pr...

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/49 Hi, @shroman , your worry make sense, but if there are some exceptions in `start` process, the JVM will exit. --- If your project is set up for it, you can reply to this email and have

[GitHub] incubator-rocketmq issue #40: [ROCKETMQ-40]Ack mode support for consume conc...

2017-02-24 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/40 @Jaskey , could you please add the discuss mail thread to here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] incubator-rocketmq pull request #64: [ROCKETMQ-102] When shutdown(), the per...

2017-02-28 Thread zhouxinyu
Github user zhouxinyu commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/64#discussion_r103434431 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/ConsumeMessageOrderlyService.java --- @@ -92,10 +92,22 @@ public void

[GitHub] incubator-rocketmq issue #69: [ROCKETMQ-111] fix possible MQClientException ...

2017-02-28 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/69 @shroman , this PR will be merged soon, looking forward to your unit tests..😀😀😀😀 --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] incubator-rocketmq issue #65: [ROCKETMQ-104] Make MQAdmin commands throw exc...

2017-02-28 Thread zhouxinyu
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/65 Hi, @shroman this PR will be merged to `develop` branch firstly as our new branching model: http://rocketmq.incubator.apache.org/docs/branching-model --- If your project is set up for

  1   2   >