You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by revans2 <gi...@git.apache.org> on 2015/11/03 16:33:25 UTC
[GitHub] storm pull request: YSTORM-162: Load Aware Shuffle Grouping
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/847
YSTORM-162: Load Aware Shuffle Grouping
I am recreating this pull request. Originally there was discussion about using the velocity of a bolt in addition to the queue size in determining the routing, but I didn't have time to implement that, and we have been running with this code in production for quite a while, so I decided to just put it up, as it has a really positive impact. This is most pronounced on heterogeneous clusters where not all nodes have the same CPU. But also this improves the performance of the groupings implementations significantly dropping the 99th and 99.9th %-lie latencies significantly for the Latency vs Throughput test case.
I would still like to explore using velocity and network distance in the routing calculations, but in a follow on JIRA.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/incubator-storm STORM-162
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/847.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 #847
----
commit 579ec324fc6172b61ef7fd3d14a532c1642f99fb
Author: Bobby Evans <ev...@yahoo-inc.com>
Date: 2015-02-10T15:07:21Z
YSTORM-162: Load Aware Shuffle Grouping
----
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/847#discussion_r43794526
--- Diff: storm-core/src/clj/backtype/storm/daemon/executor.clj ---
@@ -44,51 +45,53 @@
(defn- mk-fields-grouper [^Fields out-fields ^Fields group-fields ^List target-tasks]
(let [num-tasks (count target-tasks)
task-getter (fn [i] (.get target-tasks i))]
- (fn [task-id ^List values]
+ (fn [task-id ^List values load]
(-> (.select out-fields group-fields values)
tuple/list-hash-code
(mod num-tasks)
task-getter))))
-(defn- mk-shuffle-grouper [^List target-tasks]
- (let [choices (rotating-random-range target-tasks)]
- (fn [task-id tuple]
- (acquire-random-range-id choices))))
-
(defn- mk-custom-grouper [^CustomStreamGrouping grouping ^WorkerTopologyContext context ^String component-id ^String stream-id target-tasks]
(.prepare grouping context (GlobalStreamId. component-id stream-id) target-tasks)
- (fn [task-id ^List values]
- (.chooseTasks grouping task-id values)
- ))
+ (if (instance? LoadAwareCustomStreamGrouping grouping)
+ (fn [task-id ^List values load]
+ (.chooseTasks grouping task-id values load))
--- End diff --
Minor nit: indention.
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/847#issuecomment-154466432
LGTM. +1. This is helpful in managing load across multiple instances of component.
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/847#discussion_r43797156
--- Diff: storm-core/src/clj/backtype/storm/daemon/worker.clj ---
@@ -310,6 +313,26 @@
[node (Integer/valueOf port-str)]
))
+(defn mk-refresh-load [worker]
+ (let [local-tasks (set (:task-ids worker))
+ remote-tasks (set/difference (worker-outbound-tasks worker) local-tasks)
+ short-executor-receive-queue-map (:short-executor-receive-queue-map worker)
+ next-update (atom 0)]
+ (fn this
+ ([]
+ (let [^LoadMapping load-mapping (:load-mapping worker)
+ local-pop (map-val (fn [queue]
+ (let [q-metrics (.getMetrics queue)]
+ (/ (double (.population q-metrics)) (.capacity q-metrics))))
+ short-executor-receive-queue-map)
+ remote-load (reduce merge (for [[np conn] @(:cached-node+port->socket worker)] (into {} (.getLoad conn remote-tasks))))
+ now (System/currentTimeMillis)]
+ (.setLocal load-mapping local-pop)
+ (.setRemote load-mapping remote-load)
+ (when (> now @next-update)
+ (.sendLoadMetrics (:receiver worker) local-pop)
+ (reset! next-update (+ 5000 now))))))))
--- End diff --
We may bind the "5000" as a value in let[].
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/847#issuecomment-153485978
@zhuoliu addressed the review comments
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/847#issuecomment-153427884
@zhuoliu yes sorry typo I had a Y in front of it. I'll edit the commit message and push it out shortly.
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/847
---
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: YSTORM-162: Load Aware Shuffle Grouping
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/847#issuecomment-153415056
JIRA name is STORM-162?
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/847#issuecomment-154419473
Addressed the review comments.
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by redsanket <gi...@git.apache.org>.
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/847#discussion_r43963775
--- Diff: storm-core/src/clj/backtype/storm/daemon/executor.clj ---
@@ -44,51 +45,53 @@
(defn- mk-fields-grouper [^Fields out-fields ^Fields group-fields ^List target-tasks]
(let [num-tasks (count target-tasks)
task-getter (fn [i] (.get target-tasks i))]
- (fn [task-id ^List values]
+ (fn [task-id ^List values load]
(-> (.select out-fields group-fields values)
tuple/list-hash-code
(mod num-tasks)
task-getter))))
-(defn- mk-shuffle-grouper [^List target-tasks]
- (let [choices (rotating-random-range target-tasks)]
- (fn [task-id tuple]
- (acquire-random-range-id choices))))
-
(defn- mk-custom-grouper [^CustomStreamGrouping grouping ^WorkerTopologyContext context ^String component-id ^String stream-id target-tasks]
(.prepare grouping context (GlobalStreamId. component-id stream-id) target-tasks)
- (fn [task-id ^List values]
- (.chooseTasks grouping task-id values)
- ))
+ (if (instance? LoadAwareCustomStreamGrouping grouping)
+ (fn [task-id ^List values load]
+ (.chooseTasks grouping task-id values load))
+ (fn [task-id ^List values load]
+ (.chooseTasks grouping task-id values))))
+
+(defn mk-shuffle-grouper [^List target-tasks topo-conf ^WorkerTopologyContext context ^String component-id ^String stream-id]
--- End diff --
Should we have the arg list in the next line since it has many arguments to be more readable?
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/847#issuecomment-153467046
Just some minor nits, otherwise looks great to me. BTW, This feature is also of very good research value.
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by redsanket <gi...@git.apache.org>.
Github user redsanket commented on a diff in the pull request:
https://github.com/apache/storm/pull/847#discussion_r43963818
--- Diff: storm-core/src/clj/backtype/storm/daemon/executor.clj ---
@@ -102,7 +105,7 @@
:direct
)))
-(defn- outbound-groupings [^WorkerTopologyContext worker-context this-component-id stream-id out-fields component->grouping]
+(defn- outbound-groupings [^WorkerTopologyContext worker-context this-component-id stream-id out-fields component->grouping topo-conf]
--- End diff --
arg-list next line?
---
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: STORM-162: Load Aware Shuffle Grouping
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/847#issuecomment-153487574
+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.
---