You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by spelliott <gi...@git.apache.org> on 2014/06/30 12:36:12 UTC
[GitHub] incubator-storm pull request: Kafka spout: throw RuntimeException ...
GitHub user spelliott opened a pull request:
https://github.com/apache/incubator-storm/pull/170
Kafka spout: throw RuntimeException if a leader cannot be found for a partition
If one of the partitions of the Kafka queue has no leader available, DynamicBrokersReader.getLeaderFor(partition) returns -1. This is logged in DynamicBrokersReader.getBrokerInfo (as there's no ZK node for a broker id -1) but there is no further action.
In this situation the spout will carry on emitting only for those partitions that are available.
My assumption is that if some partitions for a queue are unavailable, there's a problem that needs to be addressed before processing continues. Happy to discuss further.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/spelliott/incubator-storm kafka-no-leader
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-storm/pull/170.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 #170
----
commit 974f0f377a371db6a673c59e13a9640fbdd9d92a
Author: Stephen Elliott <st...@qubitdigital.com>
Date: 2014-06-30T10:25:49Z
Throw RuntimeException if a leader cannot be found for a partition
----
---
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] incubator-storm pull request: [STORM-380] Kafka spout: throw Runti...
Posted by spelliott <gi...@git.apache.org>.
Github user spelliott commented on the pull request:
https://github.com/apache/incubator-storm/pull/170#issuecomment-50136101
Have added a catch to avoid the doubly nested RuntimeException, thanks d2r.
---
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] incubator-storm pull request: [STORM-380] Kafka spout: throw Runti...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-storm/pull/170
---
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] incubator-storm pull request: [STORM-380] Kafka spout: throw Runti...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/incubator-storm/pull/170#issuecomment-48561073
+1
---
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] incubator-storm pull request: [STORM-380] Kafka spout: throw Runti...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/incubator-storm/pull/170#issuecomment-51529148
+1
---
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] incubator-storm pull request: [STORM-380] Kafka spout: throw Runti...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/170#discussion_r15375343
--- Diff: external/storm-kafka/src/jvm/storm/kafka/DynamicBrokersReader.java ---
@@ -116,6 +116,9 @@ private int getLeaderFor(long partition) {
byte[] hostPortData = _curator.getData().forPath(topicBrokersPath + "/" + partition + "/state");
Map<Object, Object> value = (Map<Object, Object>) JSONValue.parse(new String(hostPortData, "UTF-8"));
Integer leader = ((Number) value.get("leader")).intValue();
+ if (leader == -1) {
+ throw new RuntimeException("No leader found for partition " + partition);
+ }
--- End diff --
It would be nice to add a catch for this so that we do not throw a doubly-wrapped RuntimeException.
...
```Java
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
```
...
---
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.
---