You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by WolfeeTJ <gi...@git.apache.org> on 2017/07/13 06:55:44 UTC
[GitHub] storm pull request #2206: STORM-2625: reduce uncommitted count when kafka co...
GitHub user WolfeeTJ opened a pull request:
https://github.com/apache/storm/pull/2206
STORM-2625: reduce uncommitted count when kafka consumer group re-ass…
STORM-2625: reduce uncommitted count when kafka consumer group re-assign partitions
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/WolfeeTJ/storm STORM-2625
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2206.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2206
----
commit 7793e4d43610c1f21681e11907cadf99f82af80c
Author: Guang Du <gu...@quarkfinance.com>
Date: 2017-07-13T06:54:23Z
STORM-2625: reduce uncommitted count when kafka consumer group re-assign partitions
----
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2206: STORM-2625: reduce uncommitted count when kafka consumer ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2206
@srdo
Thanks for noticing.
Unfortunately #2190 seemed to touch diverged thing between 1.x-branch (1.2.0-SNAPSHOT) and 1.1.x-branch (1.1.1-SNAPSHOT). I was a bit careless at merging phase while cherry-picking.
I'll see how I can fix the thing, or just revert it only from 1.1.x-branch if it's hard to fix.
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2206: STORM-2625: reduce uncommitted count when kafka consumer ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2206
Forgot to mention: Thanks for the contribution. :)
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2206: STORM-2625: reduce uncommitted count when kafka co...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2206
---
[GitHub] storm issue #2206: STORM-2625: reduce uncommitted count when kafka consumer ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2206
@WolfeeTJ
Please craft a pull request against master branch unless the patch is only valid for specific branch(es).
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2206: STORM-2625: reduce uncommitted count when kafka consumer ...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2206
@HeartSaVioR Thanks. The extra parameter it's complaining about on ZkCoordinator is the task id added in https://github.com/apache/storm/pull/2188. I think you can just remove it from the test in 1.1.x and it'll work.
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2206: STORM-2625: reduce uncommitted count when kafka consumer ...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2206
The build error is unrelated to this change, I think the cherry pick of https://github.com/apache/storm/pull/2195/files to 1.1.x-branch broke it. @HeartSaVioR if you get the chance could you take a look at it?
I'm of the opinion that it's too hard to enforce a maxUncommittedOffset globally for all partitions (we keep hitting edge cases where either the limit is ignored, or the spout is prevented from progressing), so I'd like to see us move to a per-partition limit (PR here https://github.com/apache/storm/pull/2156). That PR proposes moving numUncommittedOffsets into the OffsetManager, which means it automatically gets nulled when a partition is reassigned.
Basically I'd like to see if we can get https://github.com/apache/storm/pull/2156 discussed first. If we decide not to merge that one, then I think this is probably a good change, and I'd be happy to test this out then.
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2206: STORM-2625: reduce uncommitted count when kafka consumer ...
Posted by WolfeeTJ <gi...@git.apache.org>.
Github user WolfeeTJ commented on the issue:
https://github.com/apache/storm/pull/2206
@srdo please help to review. I tested locally it should be working. However during Travis CI build another irrelevant test case failed, I think we could safely ignore it. Please check. Thanks.
[ERROR] /home/travis/build/apache/storm/external/storm-kafka/src/test/org/apache/storm/kafka/PartitionManagerTest.java:[81,23] no suitable constructor found for ZkCoordinator(org.apache.storm.kafka.DynamicPartitionConnections,java.util.Map<java.lang.String,java.lang.Object>,org.apache.storm.kafka.SpoutConfig,org.apache.storm.kafka.ZkState,int,int,int,java.lang.String)
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2206: STORM-2625: reduce uncommitted count when kafka consumer ...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2206
@srdo Thanks I just fixed and pushed the patch.
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2206: STORM-2625: reduce uncommitted count when kafka consumer ...
Posted by WolfeeTJ <gi...@git.apache.org>.
Github user WolfeeTJ commented on the issue:
https://github.com/apache/storm/pull/2206
@srdo I agree it would be a better solution to maintain uncommitted as per partition basis, it would be much clearer to debug. Thank you very much and looking forward to merging in master or branch 1.1.x.
---
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 feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---