You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by ernisv <gi...@git.apache.org> on 2017/02/14 15:48:21 UTC
[GitHub] storm pull request #1940: STORM-2361 Kafka spout - after leader change, it s...
GitHub user ernisv opened a pull request:
https://github.com/apache/storm/pull/1940
STORM-2361 Kafka spout - after leader change, it stops committing offsets to ZK
Try to route acks/fails to recreated partition managers if manager is not found by exact (broker, partition, topic) combination
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ernisv/storm kafka-spout-leader-change-bug-2361
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1940.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 #1940
----
commit fe141fd79a0c7f1ec9a191359f9ccd6eb7a34f4b
Author: Ernestas Vaiciukevicius <e....@adform.com>
Date: 2017-02-14T15:41:02Z
Try to route acks/fails to recreated partition managers if manager is not found by exact (broker, partition, topic) combination
----
---
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 #1940: STORM-2361 Kafka spout - after leader change, it stops co...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1940
+1
@ernisv Could you squash commits into one?
Btw, I feel this patch should be along with STORM-2296, so if we would want to port this back to 1.0.x, STORM-2296 should be ported back first.
---
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 #1940: STORM-2361 Kafka spout - after leader change, it stops co...
Posted by ernisv <gi...@git.apache.org>.
Github user ernisv commented on the issue:
https://github.com/apache/storm/pull/1940
Squashed the commits.
About moving to STORM-2296 - I've put the links/relations to STORM-2296.
But as the issue itself is already in "resolved" state - not sure how the patch could be added to it.
Is registering the relation not enough?
---
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 #1940: STORM-2361 Kafka spout - after leader change, it stops co...
Posted by ernisv <gi...@git.apache.org>.
Github user ernisv commented on the issue:
https://github.com/apache/storm/pull/1940
Ok, changed the title
---
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 #1940: STORM-2361 Kafka spout - after leader change, it stops co...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1940
@ernisv
Could you change your commit title to be meaningful? We often set commit title to same as JIRA title.
I can take care of other things so you don't need to worry about that.
---
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 #1940: STORM-2361 Kafka spout - after leader change, it s...
Posted by ernisv <gi...@git.apache.org>.
Github user ernisv commented on a diff in the pull request:
https://github.com/apache/storm/pull/1940#discussion_r105104946
--- Diff: external/storm-kafka/src/jvm/org/apache/storm/kafka/KafkaSpout.java ---
@@ -172,6 +187,12 @@ public void fail(Object msgId) {
PartitionManager m = _coordinator.getManager(id.partition);
if (m != null) {
m.fail(id.offset);
+ } {
--- End diff --
Right - fixed.
---
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 #1940: STORM-2361 Kafka spout - after leader change, it stops co...
Posted by ernisv <gi...@git.apache.org>.
Github user ernisv commented on the issue:
https://github.com/apache/storm/pull/1940
Sure - braces added.
---
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 #1940: STORM-2361 Kafka spout - after leader change, it stops co...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1940
@ernisv Can you update the code to put all for, if-else, etc. blocks curly braces?
---
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 #1940: STORM-2361 Kafka spout - after leader change, it s...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1940
---
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 #1940: STORM-2361 Kafka spout - after leader change, it s...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1940#discussion_r105013260
--- Diff: external/storm-kafka/src/jvm/org/apache/storm/kafka/KafkaSpout.java ---
@@ -172,6 +187,12 @@ public void fail(Object msgId) {
PartitionManager m = _coordinator.getManager(id.partition);
if (m != null) {
m.fail(id.offset);
+ } {
--- End diff --
missing `else` between `} {`
---
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.
---