[GitHub] incubator-rocketmq pull request #2: scanResponseTable callback shoule be inv...

2016-12-21 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/2 scanResponseTable callback shoule be invoked in an executor rather than in timer thread. For success callback, processResponseCommand will execute invoke callback in a saparate executor

[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 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 #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 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. But

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

2017-01-15 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/40 @shroman how do I find the dem mailing list and get involved --- 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 #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 to

[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 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 #61: Fix risk of unable to release putMessage Lock ...

2017-02-14 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/61 @zhouxinyu , please review the updated 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 does not have

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

2017-02-15 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/40 @shroman email has been sent , please also review my imlementation and any suggestions are welcome --- If your project is set up for it, you can reply to this email and have your reply

[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-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 #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 be

[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

[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 this

[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 on

[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 to

[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 a

[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 #53: [ROCKETMQ-80] Add batch feature

2017-02-16 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/53 @dongeforever I know your implementation.What I suggest is that you add batch id for this so that we can inditify them, which is actully a very minimum effort for you. And as I

[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 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 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 #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 pull request #67: [ROCKETMQ-67] Consistent Hash allocate ...

2017-02-20 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/67#discussion_r102125928 --- 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 #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 into

[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 t

[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 I guess you still not understand my concern or I don't get it, you may try post your suggested ways and lets discuss it , a simpler solution is certainlly better if we

[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 to this

[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 pull request #64: [ROCKETMQ-102] When shutdown(), the per...

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

[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-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 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 for

[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 #66: [ROCKETMQ-106] Add flow control on topic level

2017-02-26 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/66 @zhouxinyu unit test has been added. @vongosling @lizhanhui please help review the pr, thanks! --- If your project is set up for it, you can reply to this email and have

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

2017-02-26 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/66 @lizhanhui If we really need that. We can set the defaultValue to Long.MAX_VALUE. But I think as long as we set a apporiate value, flow control should be needed to protect

[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 in

[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 pull request #53: [ROCKETMQ-80] Add batch feature

2017-03-01 Thread Jaskey
Github user Jaskey commented on a diff in the pull request: https://github.com/apache/incubator-rocketmq/pull/53#discussion_r103854335 --- Diff: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java --- @@ -331,7 +331,7 @@ public long getMinOffsetInQueue

[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 #72: [ROCKETMQ-134]the offset of message filter by ...

2017-03-08 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/72 You are right , but this is actually not a very big problem since though the offset this round is not committed, the pull request for next round is still continue so next time the offset

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

2017-03-09 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/66 @zhouxinyu @lizhanhui what about i new a config call flowControlTreadholds and a boolean config flowControlForTopic =false(default). If they set flowControlForTopic = true

[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 be

[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 This is feature to give choice to users who cares more about latency stabilization and messages duplication. As you know, the default

[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 for

[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 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 for it

[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 #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 the

[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 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 From the existing implementations, the author are apparently try to support more than one plugin easily. Maybe we can discuss about this with a separate issue in

[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 #60: [ROCKETMQ-96]Rename some temp variable ...

2017-03-12 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 #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. If

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

2017-03-22 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/40 @lizhanhui @vongosling @shroman @zhouxinyu Hi guys, would you please review this pr and let's discuss for this new feature? IMO, this is a important and useful enhancement,

[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 to

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

2017-03-23 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/66 @zhouxinyu @lizhanhui any review or advice? --- 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 #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 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, or if the

[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. The

[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 as

[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 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 So do you mean we just remove the hard code snippet in the pr and just make it log for that? Better solution will be introduced in 4.1.x in another patches? --- If your project is set up

[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, or if the

[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 for your second point, we just disable the topic level flow control by default will do the trick. pullThresholdForTopic = Integer.MAX_VALUE; max value

[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 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 for it

[GitHub] incubator-rocketmq pull request #75: [ROCKETMQ-138]Add AuthenticationExcepti...

2017-03-27 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, or if the

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

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

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

2017-03-28 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/85 [ROCKETMQ-158]Remove logback dependency for rocketmq-tools jira: https://issues.apache.org/jira/browse/ROCKETMQ-158 You can merge this pull request into a Git repository by running

[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 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 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

[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 #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 on

[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 #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 not

[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 on

[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 is

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

2017-04-01 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/86 [ROCKETMQ-160]SendHeartBeat may not be logged in the expected period JIRA:https://issues.apache.org/jira/browse/ROCKETMQ-160 You can merge this pull request into a Git repository by

[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 it

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

2017-04-10 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/75 It seems this pr is not merged yet, so I reopen 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 project does not

[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 pull request #75: [ROCKETMQ-138]Remove hard coded Aliyun ...

2017-04-10 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, or if the

[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. If

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

2017-04-11 Thread Jaskey
GitHub user Jaskey opened a pull request: https://github.com/apache/incubator-rocketmq/pull/90 [ROCKETMQ-172]log improvement for rocketmq client JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-172?jql=project%20%3D%20ROCKETMQ You can merge this pull request into a Git

[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 be

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

2017-04-11 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/28 Cool enhancement for this. But just one thing to confirm, actually rocketmq will delete commit log when the disk space is ready to be full(85% by default) even though the commit

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

2017-04-12 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/28 As far as I know, rocketmq will clean the unexpired files as long as space is 85% full and cleanFileForciblyEnable=true(true by default) Please refer to the code snippet

[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 #91: [ROCKETMQ-174]Fix spelling errors

2017-04-13 Thread Jaskey
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/91 @zhouxinyu @vesense for some public class or enum, I don't agree we rename it directly since it will introduce the compatibility problem for the user who upgrade from the

[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_r111211660 --- Diff: client/src/main/java/org/apache/rocketmq/client/producer/LocalTransactionState.java --- @@ -19,5 +19,5 @@ public enum

[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 #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

  1   2   3   >