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

2017-02-15 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/53 @dongeforever I have the same wishes for batch send too, but what drives me is that user may propably need a batch id for one batch of message, and these message should

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

2017-02-15 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/63 @lizhanhui , please review the updated 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 pull request #64: [ROCKETMQ-102] When shutdown(), the per...

2017-02-15 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/64 [ROCKETMQ-102] When shutdown(), the persisted offet is not the latest consumed message, which may cause repeated messages. Solution: add interface for push consumer to accept await

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

2017-02-15 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/63 @lizhanhui , # 4 is not the nessary conditions, since even it is not enble, the tpInfo's method is still used. try { int index

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

2017-02-16 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/66 [ROCKETMQ-106] Add flow control on topic level https://issues.apache.org/jira/browse/ROCKETMQ-106 You can merge this pull request into a Git repository by running: $ git pull https

[GitHub] incubator-rocketmq issue #69: fix possible MQClientException when query mess...

2017-02-23 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/69 Hi @yilingfeng , please modify your PR subject and mention your jira address in the description, you can follow this PR. #5 --- If your project is set up for it, you can reply

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

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

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

2017-02-24 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/40 @zhouxinyu --- 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 #68: [ROCKETMQ-107] fix possible concurrency proble...

2017-02-23 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/68 @Ah39 The problem is not only the synchrinaztion on state's read/write, but on `shutdown` and `start` , `shutdown` and `start` should be syncronized For example, two thread

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

2017-02-19 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/67 [ROCKETMQ-67] Consistent Hash allocate strategy JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-67 suport virtual nodes and custom hash algorithm You can merge this pull

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

2017-02-19 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/64 @lizhanhui @zhouxinyu please review the updated pr, which remains the same interface of push consumer. --- If your project is set up for it, you can reply to this email and have

[GitHub] incubator-rocketmq pull request #68: [ROCKETMQ-107] fix possible concurrency...

2017-02-22 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/68 [ROCKETMQ-107] fix possible concurrency problem on ServiceState when consumer start/shutdown JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-107 You can merge this pull request

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

2017-02-13 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/60 [ROCKETMQ-96]Rename some temp variable and field JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-96 You can merge this pull request into a Git repository by running: $ git pull

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

2017-02-13 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/31 I agreee with @Ah39 , 0ms is wait forever, which should not cause high cpu problem, any other justice reason to modify it to 10ms? --- If your project is set up for it, you can reply

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

2017-02-14 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/61 Fix risk of unable to release putMessage Lock forever. JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-98 You can merge this pull request into a Git repository by running: $ git

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

2017-02-16 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/64 @lizhanhui please refer to the test case and just switch the `shutdown(long milis)` method to `shutdown()` to find out the problem --- If your project is set up for it, you can reply

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

2017-02-16 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/64 @lizhanhui @zhouxinyu @vintagewang What do you think my proposol of adding a field called `awaitMilisWhenShutdown` in `DefaultPushConsumer`, which will not need to add

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

2017-02-16 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/64 @lizhanhui The problem exists for long, please review an old issue. https://github.com/alibaba/RocketMQ/issues/367 The number of duplicated messages are depending

[GitHub] incubator-rocketmq pull request #63: [ROCKETMQ-101]Fix possible NullPointerE...

2017-02-15 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/63 [ROCKETMQ-101]Fix possible NullPointerException when retry in send Async way JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-101 You can merge this pull request into a Git repository

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

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

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

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

[GitHub] incubator-rocketmq pull request #61: [ROCKETMQ-98]Fix risk of unable to rele...

2017-02-17 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/61#discussion_r101707096 --- Diff: common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java --- @@ -47,6 +47,7 @@ private boolean

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

2017-02-27 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/69 From java doc, 0 of `HOUR` is representing noon or midnight. So this problem exists if we call the method after 12:am, the HOUR will becomes 12 RATHER than 0, which may result

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

2017-02-26 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/61 @lizhanhui Acutully , if thread numbers are large , the cpu problem should be obvious without doubt in a spin lock with `while(true)` without `sleep`. But I also agree

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

2017-02-26 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/68 @shroman Actully setServiceState should be synchroinzed in my opnion, say if I am trying to shutdown, and some other thread B is trying to setServiceState ,which should be waiting

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

2017-02-26 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/68 @shroman @vongosling Do all guys consider makeing method synchrinized which is locking the consumer object is a better way instead of a more fine-grained lock. If so, I

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

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

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

2017-02-28 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/64 @zhouxinyu Please check the updated pr. Since shutdownNow needs developer to take respond to interrupts but actually they should not care about this, so I still remain

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

2017-01-13 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/40 @zhouxinyu confict has been resolved. please review. Of course we can discuss the details. Please provide a comment box or disucss post for this so that we can follow

[GitHub] incubator-rocketmq issue #19: [ROCKETMQ-14] rocketmq-14 unit test updated

2017-01-03 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/19 @vongosling Is there any problem on this unit test code? --- 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 #19: ROCKETMQ_14 unit test updated

2016-12-27 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/19 ROCKETMQ_14 unit test updated A runnable unit test update. Create a mock server before test. You can merge this pull request into a Git repository by running: $ git pull

[GitHub] incubator-rocketmq issue #2: invoke callback shoule be invoked in an executo...

2016-12-22 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/2 @vongosling please check the commited unit 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 issue #75: [ROCKETMQ-138]Add AuthenticationException clas...

2017-03-28 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/75 @vongosling Now nothing special has been done unless remove the hard corded aliyun code snippet. I will close this pr after merge. --- If your project is set up

[GitHub] incubator-rocketmq issue #75: [ROCKETMQ-138]Add AuthenticationException clas...

2017-03-27 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/75 Do you guys really consider that is good enough?? @vongosling @lollipopjin IMO, the origin design is OK, since the AuthenticationException is thrown from the hook

[GitHub] incubator-rocketmq issue #75: [ROCKETMQ-138]Add AuthenticationException clas...

2017-03-27 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/75 @vongosling OK, look forward to that! So when will this pr be merged? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] incubator-rocketmq pull request #63: [ROCKETMQ-101]Fix possible NullPointerE...

2017-03-27 Thread Jaskey
GitHub user Jaskey reopened a pull request: https://github.com/apache/incubator-rocketmq/pull/63 [ROCKETMQ-101]Fix possible NullPointerException when retry in send Async way JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-101 You can merge this pull request into a Git

[GitHub] incubator-rocketmq pull request #63: [ROCKETMQ-101]Fix possible NullPointerE...

2017-03-27 Thread Jaskey
Github user Jaskey closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/63 --- 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 #63: [ROCKETMQ-101]Fix possible NullPointerExceptio...

2017-03-27 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/63 Thank you @vongosling @lizhanhui @zhouxinyu --- 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 #66: [ROCKETMQ-106] Add flow control on topic level

2017-03-27 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/66 @lizhanhui > Can we maintain semantics of the original config and name this specific config something like flowControlThresholdByTopic? This already done in my current

[GitHub] incubator-rocketmq issue #71: add telnet CMD to nameServer

2017-03-22 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/71 @Ah39 , do you create a brand new pr with the same issue, if so IMO, this pr can be closed and let's review the pr of #81 --- If your project is set up for it, you can reply

[GitHub] incubator-rocketmq issue #85: [ROCKETMQ-158]Remove logback dependency for ro...

2017-03-29 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/85 @lizhanhui please review the updated pr, log4j_tools.xml has been move to distributions. --- If your project is set up for it, you can reply to this email and have your reply appear

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

2017-03-29 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/61 @lizhanhui the comments has been updated and the suggestion in the previous pr has been removed, please help review. --- If your project is set up for it, you can reply to this email

[GitHub] incubator-rocketmq issue #85: [ROCKETMQ-158]Remove logback dependency for ro...

2017-03-29 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/85 @lizhanhui please review the updated 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

[GitHub] incubator-rocketmq issue #85: [ROCKETMQ-158]Remove logback dependency for ro...

2017-03-28 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/85 @lizhanhui So do you agree this is indeed an issue that needed to be solved? If yes, I will try doing that. But actually it is a little different from the clientLogger

[GitHub] incubator-rocketmq pull request #85: [ROCKETMQ-158]Remove logback dependency...

2017-03-29 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/85#discussion_r108666872 --- Diff: tools/src/main/java/org/apache/rocketmq/tools/command/MQAdminStartup.java --- @@ -196,22 +196,11 @@ private static void initLog

[GitHub] incubator-rocketmq issue #71: add telnet CMD to nameServer

2017-03-21 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/71 Please notice your pr topic and relate the JIRA in the descriptions. --- 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 #75: [ROCKETMQ-138]Remove hard coded Aliyun ...

2017-04-10 Thread Jaskey
GitHub user Jaskey reopened a pull request: https://github.com/apache/incubator-rocketmq/pull/75 [ROCKETMQ-138]Remove hard coded Aliyun authentication code JIRA:https://issues.apache.org/jira/browse/ROCKETMQ-138 You can merge this pull request into a Git repository by running

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

2017-04-01 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/68 @lizhanhui @zhouxinyu @shroman Any more advice on this pr? and when can this be merged? --- If your project is set up for it, you can reply to this email and have your reply appear

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

2017-04-01 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/64 Repeated messages is a big problem for a message queue, and this is a known issue. When can this pr be reviewed and merged? @lizhanhui @zhouxinyu @vongosling --- If your project

[GitHub] incubator-rocketmq pull request #85: [ROCKETMQ-158]Remove logback dependency...

2017-04-12 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/85#discussion_r111210164 --- Diff: common/src/main/java/org/apache/rocketmq/common/utils/LogUtils.java --- @@ -0,0 +1,96 @@ +/* + * Licensed to the Apache Software

[GitHub] incubator-rocketmq issue #67: [ROCKETMQ-67] Consistent Hash allocate strateg...

2017-04-06 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/67 @shroman @zhouxinyu @lizhanhui @vongosling I heard that consistent hash strategy will be accomplished in 4.1.x , while this pr has been open for months, can we accelerate

[GitHub] incubator-rocketmq pull request #75: [ROCKETMQ-138]Remove hard coded Aliyun ...

2017-04-11 Thread Jaskey
Github user Jaskey closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/75 --- 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 #77: [ROCKETMQ-140]Guard MQVesion methods.

2017-04-11 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/77 This issue exits for long, whose is trival but has great impact when managing rocketmq in console, which may result in the cluster page not able to render. So this pr should

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

2017-04-11 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 @lizhanhui @zhouxinyu Any guys come to review this trival changes? --- 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 #90: [ROCKETMQ-172]log improvement for rocke...

2017-04-16 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111701497 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java --- @@ -577,12 +577,12 @@ public void operationComplete

[GitHub] incubator-rocketmq pull request #90: [ROCKETMQ-172]log improvement for rocke...

2017-04-16 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111701589 --- Diff: remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java --- @@ -321,6 +321,7 @@ public void

[GitHub] incubator-rocketmq issue #90: [ROCKETMQ-172]log improvement for rocketmq cli...

2017-04-17 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 @shroman Please review again --- 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 #92: [ROCKETMQ-175] Consumer may miss messages beca...

2017-04-17 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/92 @vsair What I mean is that the scene this pr is going to fix is breaking the restrict that the same consumer group has the same subscriptions. Though this may help improve

[GitHub] incubator-rocketmq issue #90: [ROCKETMQ-172]log improvement for rocketmq cli...

2017-04-17 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/90 @dongeforever @vongosling @shroman Thank you guys! --- 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 #90: [ROCKETMQ-172]log improvement for rocke...

2017-04-17 Thread Jaskey
Github user Jaskey closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/90 --- 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 pull request #91: [ROCKETMQ-174]Fix spelling errors

2017-04-13 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/91#discussion_r111524324 --- Diff: client/src/main/java/org/apache/rocketmq/client/producer/LocalTransactionExecutor.java --- @@ -18,6 +18,6 @@ import

[GitHub] incubator-rocketmq pull request #85: [ROCKETMQ-158]Remove logback dependency...

2017-04-13 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/85#discussion_r111524822 --- Diff: common/src/main/java/org/apache/rocketmq/common/utils/LogUtils.java --- @@ -0,0 +1,96 @@ +/* + * Licensed to the Apache Software

[GitHub] incubator-rocketmq pull request #91: [ROCKETMQ-174]Fix spelling errors

2017-04-13 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/91#discussion_r111211195 --- Diff: client/src/main/java/org/apache/rocketmq/client/consumer/listener/ConsumeReturnType.java --- @@ -33,7 +33,7

[GitHub] incubator-rocketmq pull request #91: [ROCKETMQ-174]Fix spelling errors

2017-04-13 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/91#discussion_r111211864 --- Diff: client/src/main/java/org/apache/rocketmq/client/producer/LocalTransactionExecutor.java --- @@ -18,6 +18,6 @@ import

[GitHub] incubator-rocketmq pull request #90: [ROCKETMQ-172]log improvement for rocke...

2017-04-14 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111552422 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java --- @@ -577,12 +577,12 @@ public void operationComplete

[GitHub] incubator-rocketmq issue #92: [ROCKETMQ-175] Consumer may miss messages beca...

2017-04-14 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/92 Actually missing message can not be avoided when the diffrerent tags are subscribed in the same consumer groups. Say for two queues q1 q2, two consumers c1 c2. c1->TAG1

[GitHub] incubator-rocketmq pull request #95: [ROCKETMQ-184]-It takes too long(3-33 s...

2017-04-20 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/95 [ROCKETMQ-184]-It takes too long(3-33 seconds) to switch to read from slave when master crashes JIRA:https://issues.apache.org/jira/browse/ROCKETMQ-184?jql=project%20%3D%20ROCKETMQ

[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...

2017-04-21 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 @lizhanhui I have considered that, which will takes more efforts to achieve the same goal. Since I need to change some structure to make the connect manager to get access

[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...

2017-03-10 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/73 @shroman user may have several plugins as Decorator Patterns, and `build` method returns the outtest store decorator, which in my opinion is right. --- If your project is set up

[GitHub] incubator-rocketmq pull request #75: Add AuthenticationException class to re...

2017-03-10 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/75 Add AuthenticationException class to remove hard coded Aliyun authentication code JIRA:https://issues.apache.org/jira/browse/ROCKETMQ-138 You can merge this pull request into a Git

[GitHub] incubator-rocketmq pull request #75: Add AuthenticationException class to re...

2017-03-10 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/75#discussion_r105354854 --- Diff: remoting/src/main/java/org/apache/rocketmq/remoting/exception/AuthenticationException.java --- @@ -0,0 +1,26

[GitHub] incubator-rocketmq issue #67: [ROCKETMQ-67] Consistent Hash allocate strateg...

2017-03-09 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/67 @shroman The detail descriptions has been updated to JIRA. I will try described as detail as possible when I create an issue in JIRA in the future. --- If your project is set up

[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...

2017-03-10 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/73 @shroman Sorry, I still cannot see the problem. No initialization are called inside the build method and only the outer class are returned. Do you concern about

[GitHub] incubator-rocketmq issue #67: [ROCKETMQ-67] Consistent Hash allocate strateg...

2017-03-09 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/67 @zhouxinyu @lizhanhui @shroman @vongosling any advice on this pr? Since I think consistent hash can be also applied to order message shading, the classes in this pr can

[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...

2017-03-12 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/73 @shroman , indeed it works.But this can be.done by rocketmq actually to make it easier to make up.everything. Say user may have to plugin two store pligins , one for mysql, one

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

2017-03-13 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/60#discussion_r105594848 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/producer/DefaultMQProducerImpl.java --- @@ -449,9 +449,9 @@ private SendResult

[GitHub] incubator-rocketmq issue #60: [ROCKETMQ-96]Rename some temp variable and fie...

2017-03-12 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/60 @zhouxinyu @shroman @vongosling @lizhanhui Guys, how do you think of this pr, which does not change any functionality , I just rename some variables/fields to make code more readable

[GitHub] incubator-rocketmq issue #75: [ROCKETMQ-138]Add AuthenticationException clas...

2017-03-12 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/75 @vongosling @shroman please review the updated 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

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

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

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

2017-03-30 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/61 @zhouxinyu @shroman @vongosling please 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

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

2017-04-18 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 @zhouxinyu , hmm, it depends on what we need. The issue I post is that , the log is not doing as expected when the addr table is empty. Even though log for all broker is reasonable

[GitHub] incubator-rocketmq issue #86: [ROCKETMQ-160]SendHeartBeat may not be logged ...

2017-04-18 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/86 @shroman For your first point, I would like to remain the existing flow? Because the heart beat sent may be failed and if we only mark the success one, we maybe miss the log

[GitHub] incubator-rocketmq pull request #68: [ROCKETMQ-107] fix possible concurrency...

2017-04-18 Thread Jaskey
Github user Jaskey closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/68 --- 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 #68: [ROCKETMQ-107] fix possible concurrency proble...

2017-04-18 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/68 Thank you @zhouxinyu --- 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 #85: [ROCKETMQ-158]Remove logback dependency for ro...

2017-04-19 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/85 @zhouxinyu @shroman @vongosling what's your advice guys? --- 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 #139: [ROCKETMQ-242] mqclient can not fetch ...

2017-08-02 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/139#discussion_r131043088 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/factory/MQClientInstance.java --- @@ -255,19 +255,17 @@ public void start

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

2017-08-02 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/138 @lyy4j please see https://github.com/apache/incubator-rocketmq/blob/0c5e53db6f4d0ed9f25747379a8b679e2da5392d/common/src/main/java/org/apache/rocketmq/common/consistenthash

[GitHub] incubator-rocketmq pull request #147: [ROCKETMQ-266] Add a specific Exceptio...

2017-08-15 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/147#discussion_r133349431 --- Diff: client/src/test/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPushConsumerImplTest.java --- @@ -0,0 +1,48

[GitHub] incubator-rocketmq issue #144: [ROCKETMQ-257] name server address and web se...

2017-08-13 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/144 Actually, the existing design is that : we have a default name server service address, which will be specified in our doc(though it is indeed not demonstrated well for now), the default

[GitHub] incubator-rocketmq issue #144: [ROCKETMQ-257] name server address and web se...

2017-08-13 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/144 @qqeasonchen also, please notice your tittle to make it complete. And link the jira issue in the desciption --- If your project is set up for it, you can reply to this email and have

[GitHub] incubator-rocketmq pull request #147: [ROCKETMQ-266] Add a specific Exceptio...

2017-08-17 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/147#discussion_r133633277 --- Diff: client/src/test/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPushConsumerImplTest.java --- @@ -0,0 +1,48

[GitHub] incubator-rocketmq pull request #130: [ROCKETMQ-243] BrokerData#selectBroker...

2017-07-12 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/130#discussion_r126865859 --- Diff: common/src/main/java/org/apache/rocketmq/common/protocol/route/BrokerData.java --- @@ -37,15 +41,21 @@ public BrokerData(String

[GitHub] incubator-rocketmq issue #130: [ROCKETMQ-243] BrokerData#selectBrokerAddr() ...

2017-07-10 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/130 @shroman Sorry I misunderstand this method that I though it returns the master address, since it is prefered to return the master address then slave, renaming seems to be not necessary

[GitHub] incubator-rocketmq issue #131: [ROCKETMQ-244] BrokerData#selectBrokerAddr() ...

2017-07-11 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/131 The title is pointing to another issue, please modify it when you are convenient. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] incubator-rocketmq-site issue #20: Add committer Jaskey to team site

2017-07-26 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq-site/pull/20 Done --- 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 pull request #127: [ROCKETMQ-233] Fix pull interval issue

2017-06-28 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/127#discussion_r124174720 --- Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPushConsumerImpl.java --- @@ -273,6 +273,17 @@ public void

[GitHub] incubator-rocketmq issue #128: [ROCKETMQ-234] bugfix - broker will write res...

2017-06-28 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/128 @evthoriz please update the jira's discriptions so that the user knows this bug and determine whether they should upgrade --- If your project is set up for it, you can reply

[GitHub] incubator-rocketmq pull request #126: [ROCKETMQ-231] fix pull consumer pull ...

2017-07-04 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/126#discussion_r125448602 --- Diff: store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java --- @@ -1110,7 +1110,7 @@ private boolean isTheBatchFull(int

[GitHub] incubator-rocketmq pull request #110: [ROCKETMQ-208]incompatibility problem ...

2017-07-03 Thread Jaskey
Github user Jaskey closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/110 --- 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 #119: [ROCKETMQ-223]-Rename DEFAULT_TOPIC

2017-07-05 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/119 @lizhanhui @dongeforever any opinons? Can this pr be merged? please review --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

  1   2   >