[GitHub] storm issue #2164: [STORM-2557]: A bug in DisruptorQueue causing severe unde...

2017-10-12 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/2164
  
@HeartSaVioR @asfgit It seems that we forgot to add @wendyshusband to the 
contributor list.


---

[GitHub] storm issue #2164: [STORM-2557]: A bug in DisruptorQueue causing severe unde...

2017-06-19 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/2164
  
@revans2 I quite agree with you. The current publish logic in disruptor 
queue is difficult to follow. It will be great if some one could refine the 
code in a separate JIRA, so that we can easily know what are storing in the 
queue.

@HeartSaVioR Given the feedback from revans2, could you merge this 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 feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #2164: [STORM-2557]: A bug in DisruptorQueue causing severe unde...

2017-06-18 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/2164
  
+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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1719: [STORM-1057] Add throughput metrics to spouts/bolts and d...

2016-11-22 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/1719
  
@HeartSaVioR, @revans2, @harshach,@d2r,@unsleepy22 Any comment to this PR, 
please?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1772: [STORM-2196] A typo in RAS_Node::consumeCPU

2016-11-09 Thread wangli1426 
GitHub user wangli1426 opened a pull request:

https://github.com/apache/storm/pull/1772

[STORM-2196] A typo in RAS_Node::consumeCPU

Fixed a typo in RAS_Node::consumeCPU()

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/wangli1426/storm patch-2

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/1772.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 #1772


commit 2bae6789a47b2cb848c844a8a3e552df8e65f97d
Author: Li Wang <wangli1...@gmail.com>
Date:   2016-11-10T04:48:26Z

[STORM-2196] A typo in RAS_Node::consumeCPU

Fixed a typo in RAS_Node::consumeCPU()




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1763: [STORM-2189] RAS_Node::freeCPU outputs incorrect i...

2016-11-06 Thread wangli1426 
GitHub user wangli1426 opened a pull request:

https://github.com/apache/storm/pull/1763

[STORM-2189] RAS_Node::freeCPU outputs incorrect info

CPU trying to free should be ``_availCPU + amount`` rather than 
``_availMemory + amount``.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/wangli1426/storm patch-1

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/1763.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 #1763


commit 7f0479fba3c7c97288a8cadc856cfd1dfda3ca4a
Author: Li Wang <wangli1...@gmail.com>
Date:   2016-11-07T03:03:05Z

[STORM-2189] RAS_Node::freeCPU outputs incorrect info

CPU trying to free should be ``_availCPU + amount`` rather than 
``_availMemory + amount``.




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1719: [STORM-1057] Add throughput metrics to spouts/bolts and d...

2016-10-26 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/1719
  
Any comment?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #753: [STORM-1057] Add throughput metrics to spouts/bolts and di...

2016-09-28 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/753
  
This new feature has been implemented on master branch in #1719. Please 
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 have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1719: [STORM-1057] Add throughput metrics to spouts/bolt...

2016-09-28 Thread wangli1426 
GitHub user wangli1426 opened a pull request:

https://github.com/apache/storm/pull/1719

[STORM-1057] Add throughput metrics to spouts/bolts and display them on web 
ui for master branch

Hi @HeartSaVioR, @revans2, @harshach,@d2r,@unsleepy22,

In this PR, throughput of tasks is calculated and displayed on web ui.
If you want to check the new feature, do not forget to clear your browser 
cache first so that the throughput columns can be displayed.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/wangli1426/storm throughput-for-master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/1719.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 #1719


commit fbb51095f7f915b9dabf2bb1d3c643b67a20a807
Author: Li Wang <wangli1...@gmail.com>
Date:   2016-09-28T05:39:16Z

create throughput metric for the bolt/spout.

commit c3f5e1cc8acd3a59bbd2f75a35e9f293942ffcd1
Author: Li Wang <wangli1...@gmail.com>
Date:   2016-09-29T02:34:43Z

display throughput metrics on web ui.

commit bba445d91ab92ff0e2a88457b119a88e9329cbc2
Author: Li Wang <wangli1...@gmail.com>
Date:   2016-09-29T02:35:15Z

avoiding losing precision when computing the aggregated throughput for 
spout/bolt.




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1703: [STORM-1057] Add throughput metrics to spouts/bolts and d...

2016-09-27 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/1703
  
Thanks for the prompt response. I will create a new pull request for master 
asap. 

Sent from my iPhone

> On 28 Sep 2016, at 9:09 AM, Jungtaek Lim <notificati...@github.com> wrote:
> 
> Yes sure. You don't essentially need to upmerge/rebase old pull request 
if you want to provide new pull request based on current.
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1703: [STORM-1057] Add throughput metrics to spouts/bolts and d...

2016-09-27 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/1703
  
Can I create a new PR based on the latest master version rather than 
upmerging this RP to master? The former would be much easier, since a lot of 
changes have been made from 1.x to 2.x. 

Sent from my iPhone

> On 28 Sep 2016, at 8:02 AM, Jungtaek Lim <notificati...@github.com> wrote:
> 
> @wangli1426 
> Sorry I haven't had time to review this right now. I'll take a look when 
I get time. 
> Btw, since you're modifying ported clojure files, you need to port your 
change to master as well. If you don't want to address both Clojure and Java, 
addressing master is higher priority.
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #1703: [STORM-1057] Add throughput metrics to spouts/bolts and d...

2016-09-26 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/1703
  
@HeartSaVioR Can you review this PR? Thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #753: [STORM-1057] Add throughput metrics to spouts/bolts and di...

2016-09-22 Thread wangli1426 
Github user wangli1426 commented on the issue:

https://github.com/apache/storm/pull/753
  
@harshach I managed to upmerge this PR to 1.x-branch in #1703. Please 
review. Thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #1703: [STORM-1057] Add throughput metrics to spouts/bolt...

2016-09-22 Thread wangli1426 
GitHub user wangli1426 opened a pull request:

https://github.com/apache/storm/pull/1703

[STORM-1057] Add throughput metrics to spouts/bolts and display them on web 
ui for 1.x-branch

Hi @HeartSaVioR, @revans2, @harshach,@d2r,@unsleepy22,

I upmerge PR apache/storm#753 to 1.x-branch.
Sorry for the delay.
Please review.

Thanks.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/wangli1426/storm throughput-metrics

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/1703.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 #1703


commit 67721f77f28cbb4a29fab7e3023b220095886338
Author: Li Wang <wangli1...@gmail.com>
Date:   2015-09-18T02:11:20Z

initializing Timer in RateTracker.java lazily to avoid a bug in Clojure 
Maven plugin that might make compiling process never finish

commit 1dad2041fb0ae8689794fec93d05df2266292dc8
Author: Li Wang <wangli1...@gmail.com>
Date:   2015-09-18T07:19:28Z

take throughput measurements on spouts and bolts

commit d74f784175cc4b8c631c52acaab3e0743f11a56d
Author: Li Wang <wangli1...@gmail.com>
Date:   2015-09-20T01:26:11Z

keep Nimbus updated to the latest throughput stats

commit 066e234bcf141525c0a1ed4a04f6d3cf160f6c28
Author: wangli1426 <wangli1...@gmail.com>
Date:   2015-09-22T14:17:17Z

display the throughput metrics on web ui

commit d552a990af56ccd7e8e5211a0146030fccf570a1
Author: wangli1426 <wangli1...@gmail.com>
Date:   2015-10-03T15:29:37Z

addressed all concerns raised by revans2

commit 22460771279c788198dbbbfa060a67db05cf449f
Author: wangli1426 <wangli1...@gmail.com>
Date:   2015-10-17T04:57:22Z

upmerged with master

commit a5116b38ee4f64e5bec951b81538b1f4a1c980bd
Author: wangli1426 <wangli1...@gmail.com>
Date:   2015-10-17T06:59:54Z

updated storm.thrift for rolling update, removed unused functions in 
stats.clj, and updated STORM-UI-REST-API.md

commit 4de33d1562bbdc2d7c7d17c605614939fe4b288e
Author: wangli1426 <wangli1...@gmail.com>
Date:   2015-10-21T15:37:59Z

Merge remote-tracking branch 'apache/master' into throughput-metrics

commit 1b0598485adb298d1e008b82ecda706d5996721f
Author: wangli1426 <wangli1...@gmail.com>
Date:   2015-10-22T02:59:06Z

addressed issues raised by revans2

commit fd89a78a9acac830cab3391da4f94eb232ff68fb
Author: Li Wang <wangli1...@gmail.com>
Date:   2016-09-22T07:21:12Z

Merge branch '1.x-branch' into throughput-metrics




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2016-03-18 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-197654319
  
@harshach Sure, I am happy to upmerge it now. Should I upmerge it for 
1.x-branch, master branch, or both?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2016-03-15 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-197118115
  
@harshach  I can up-merge this commit. But before I start, I would like to 
make sure that more committers want this PR get merged. I really spent a lot of 
time and effort in up-merging this PR last time.

By the way, should I upmerge this PR for master or only for 1.x-branch?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-23 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-150751146
  
@d2r ,
Thank you very much for your prompt response. However, I cannot quite 
understand your meaning by 
> If the previous worker's throughput stats had declined sharply 
before the worker had died, then weighting the current worker's throughput 
stats still would be inaccurate, but in a different way. 

I will appreciate it a lot if you could provide a concrete example. 

I couldn't agree with you more than storm needs a History Server keep 
historical information. Otherwise, executors are responsible for maintaining 
their stats, which make them stateful. Is there any plan about the history 
server?

By the way, adding throughput metric is my first step. And my ultimate goal 
is to add ***normalized*** throughput, which leverages queueing theory to 
provide a comparable performance metrics, similar but more accurate than 
```capacity``` that is currently available in Storm. With normalized 
throughput, one can easily identify the performance bottleneck of a running 
topology by finding the executor with minimal number in normalized throughput. 
With this capability, we can develop a runtime scheduling algorithm to make 
better resource allocation. So what do you think?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-21 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-149948793
  
Hi @revans2 ,
Recent commits to the master branch cause conflict to my PR, so I up-merged 
my PR in 4de33d1. Could you please review the code again? Thank you very much. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-21 Thread wangli1426 
Github user wangli1426 commented on a diff in the pull request:

https://github.com/apache/storm/pull/753#discussion_r42704498
  
--- Diff: storm-core/src/clj/backtype/storm/stats.clj ---
@@ -277,15 +277,34 @@
  (value-stats stats SPOUT-FIELDS)
  {:type :spout}))
 
+(defn values-divided-by [pairs t]
+  (let [update-values (fn [m f & args]
+(into {} (for [[k v] m] [k (apply f v args)])))]
+(update-values pairs / (double t
--- End diff --

Thanks for the comment. I will make ```update-values``` as a regular 
function in this revision.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-21 Thread wangli1426 
Github user wangli1426 commented on a diff in the pull request:

https://github.com/apache/storm/pull/753#discussion_r42704561
  
--- Diff: STORM-UI-REST-API.md ---
@@ -351,11 +354,13 @@ Sample response:
 "executors": 12,
 "emitted": 184580,
 "transferred": 0,
+"throughput": "195.000",
 "acked": 184640,
 "executeLatency": "0.048",
 "tasks": 12,
 "executed": 184620,
 "processLatency": "0.043",
+"throughput": 
--- End diff --

My bad. This will be solved in this revision.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-21 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-150076986
  
Hi @revans2 ,
Thanks for very much for your time and efforts. Following your suggestions, 
I have made ```update-values``` as a regular function and fixed the problem in 
```storm/STORM-UI-REST-API.md```. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-21 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-150077548
  
Hi @d2r,
Could you please review this PR? Your response is highly appreciated. 
Thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-16 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-148779028
  
@revans2 ,
Thanks for your comment. All the concerns are addressed in 99a898f. Please 
review the code for another round. Thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-16 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-148681032
  
Hi @revans2 ,
I  up-merged my code successfully. Following your suggestion, I mark 
throughput in storm.thrift as optional. Look forward to your response. Thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-16 Thread wangli1426 
Github user wangli1426 commented on a diff in the pull request:

https://github.com/apache/storm/pull/753#discussion_r42259381
  
--- Diff: storm-core/src/clj/backtype/storm/ui/core.clj ---
@@ -626,7 +638,8 @@
   (let [^CommonAggregateStats cas (.get_common_stats stats)]
 {"stream" stream-id
  "emitted" (nil-to-zero (.get_emitted cas))
- "transferred" (nil-to-zero (.get_transferred cas))}))
+ "transferred" (nil-to-zero (.get_transferred cas))
+ "throughput" (float-str (.get_throughput cas))}))
--- End diff --

Don't worry, nil value will be converted into 0 in ```float-str```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-16 Thread wangli1426 
Github user wangli1426 commented on a diff in the pull request:

https://github.com/apache/storm/pull/753#discussion_r42259569
  
--- Diff: storm-core/src/storm.thrift ---
@@ -244,8 +244,9 @@ struct CommonAggregateStats {
 2: optional i32 num_tasks;
 3: optional i64 emitted;
 4: optional i64 transferred;
-5: optional i64 acked;
-6: optional i64 failed;
+5: optional double throughput;
+6: optional i64 acked;
+7: optional i64 failed;
--- End diff --

Thanks for the comment and point taken.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-16 Thread wangli1426 
Github user wangli1426 commented on a diff in the pull request:

https://github.com/apache/storm/pull/753#discussion_r42259449
  
--- Diff: storm-core/src/clj/backtype/storm/stats.clj ---
@@ -205,6 +205,9 @@
   [stats stream amt]
   (update-executor-stat! stats [:common :transferred] stream (* 
(stats-rate stats) amt)))
 
+(defn update-stats-throughput! [stats stream throughput]
+  (update-executor-stat! stats [:common :throughput] stream (* (stats-rate 
stats) throughput)))
+
--- End diff --

Thanks for the comment, I will delete those lines.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-14 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-148289930
  
@revans2 
Thank you for your comment. I will up-merge this PR. However, I can't quite 
understand your last sentence. What do you mean by "you like to see the thrift 
code changed"? Could please explain more about it? Thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-03 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-145260599
  
@revans2 ,
I have addressed all your concerns in d552a99. The most important 
modification is that instead of employing RateTracker, I reuse the stats of 
```emitted``` and ```executed``` to generate the throughput stats for spout and 
bolt respectively.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-02 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/753#issuecomment-145194180
  
@revans2,
Thanks for your detailed comments. I understand your concern that we might 
not need to use a new system to obtain the throughput metric, given that we 
already have code in stats.clj doing the same thing. I will try to obtain the 
throughput metric using existing functions in stats.clj and come back when I 
finish. 




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-01 Thread wangli1426 
Github user wangli1426 commented on a diff in the pull request:

https://github.com/apache/storm/pull/753#discussion_r40911206
  
--- Diff: storm-core/src/jvm/backtype/storm/utils/RateTracker.java ---
@@ -72,7 +80,7 @@ public RateTracker(int validTimeWindowInMils, int 
numOfSlides, boolean simulate
  * @param count number of arrivals
  */
 public void notify(long count) {
-_histograms[_histograms.length-1]+=count;
+_histograms[_numOfSlides - 1] += count;
--- End diff --

I am glad to hear that.


> On Oct 1, 2015, at 21:09, Robert (Bobby) Evans <notificati...@github.com> 
wrote:
> 
> In storm-core/src/jvm/backtype/storm/utils/RateTracker.java 
<https://github.com/apache/storm/pull/753#discussion_r40910516>:
> 
> > @@ -72,7 +80,7 @@ public RateTracker(int validTimeWindowInMils, int 
numOfSlides, boolean simulate
> >   * @param count number of arrivals
> >   */
> >  public void notify(long count) {
> > -_histograms[_histograms.length-1]+=count;
> > +_histograms[_numOfSlides - 1] += count;
> I was planning on doing it today, because I had already pulled it into my 
distribution, and it should not be that hard to do. It is mostly switching the 
_histograms to be an array of AtomicLong instead of regular long, and then 
making sure that the current bucket is modified using getAndSet so that there 
is no read modify write that is not atomic. I was also thinking I would do 
something so that the current bucket does not require any offset calculations, 
but that is a very small optimization, that once the JIT kicks in probably 
would not make any difference.
> 
> —
> Reply to this email directly or view it on GitHub 
<https://github.com/apache/storm/pull/753/files#r40910516>.
> 




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-10-01 Thread wangli1426 
Github user wangli1426 commented on a diff in the pull request:

https://github.com/apache/storm/pull/753#discussion_r40907742
  
--- Diff: storm-core/src/jvm/backtype/storm/utils/RateTracker.java ---
@@ -72,7 +80,7 @@ public RateTracker(int validTimeWindowInMils, int 
numOfSlides, boolean simulate
  * @param count number of arrivals
  */
 public void notify(long count) {
-_histograms[_histograms.length-1]+=count;
+_histograms[_numOfSlides - 1] += count;
--- End diff --

@revans2 
Thanks for your comment. Will you solve [STORM-1078] by yourself? I can 
solve this bug if you are too busy to do 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 have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1057] Add throughput metrics to spouts/...

2015-09-22 Thread wangli1426 
GitHub user wangli1426 opened a pull request:

https://github.com/apache/storm/pull/753

[STORM-1057] Add throughput metrics to spouts/bolts and display them on web 
ui

The throughputs for the spouts and bolts could help the user to identify 
the performance bottleneck and detect the load balancing issue. In this RP, I 
take measurements on the throughput of the executors and display them on web UI.

### Summary of Changes
1. Take throughput measurements on the spouts and bolts;
2. Add throughput to ExecutorStats;
3. Display the throughputs on web UI.


**Note: If you cannot see the throughputs on your web UI, please clean your 
browser cache and try again.**
### Screenshots

![screen shot 2015-09-21 at 13 16 
01](https://cloud.githubusercontent.com/assets/5260276/9987644/3e3b5720-607c-11e5-928c-ec49892b67f6.png)
![screen shot 2015-09-21 at 13 17 
24](https://cloud.githubusercontent.com/assets/5260276/9987684/953e7048-607c-11e5-9188-b0f8ba1c7943.png)
![screen shot 2015-09-21 at 13 17 
57](https://cloud.githubusercontent.com/assets/5260276/9987686/96ab9dc0-607c-11e5-954d-2369069b22d6.png)
![screen shot 2015-09-21 at 13 18 
49](https://cloud.githubusercontent.com/assets/5260276/9987687/999e7e26-607c-11e5-99e4-0ac8d31b8768.png)

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/wangli1426/storm throughput-metrics

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/753.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 #753


commit 67721f77f28cbb4a29fab7e3023b220095886338
Author: Li Wang <wangli1...@gmail.com>
Date:   2015-09-18T02:11:20Z

initializing Timer in RateTracker.java lazily to avoid a bug in Clojure 
Maven plugin that might make compiling process never finish

commit 1dad2041fb0ae8689794fec93d05df2266292dc8
Author: Li Wang <wangli1...@gmail.com>
Date:   2015-09-18T07:19:28Z

take throughput measurements on spouts and bolts

commit d74f784175cc4b8c631c52acaab3e0743f11a56d
Author: Li Wang <wangli1...@gmail.com>
Date:   2015-09-20T01:26:11Z

keep Nimbus updated to the latest throughput stats

commit 1a2a9b38cd256b29cb0177450f29d03ce20471c3
Author: Li Wang <wangli1...@gmail.com>
Date:   2015-09-21T05:27:05Z

display throughput metrics on web UI




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1007] Added tuple arrival rate and tupl...

2015-09-17 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/716#issuecomment-141110751
  
@HeartSaVioR 

Sorry to interrupt, but may I ask when this PR could be merged? I am 
planing to display the new metrics on the web UI, if this PR gets merged. 
Thanks.



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1007] Added tuple arrival rate and tupl...

2015-09-08 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/716#issuecomment-138752669
  
@HeartSaVioR 
Thanks for voting +1.

By the way, in my original design, I didn't use timer to update the 
histograms. Instant, the histograms were updated in ```notify()```. However, 
such a design requires to call ```System.currentTimeMills()``` every time when 
```notify()``` is called, to determine whether an update to the histograms is 
needed. ```System.currentTimeMills()``` is fairly expensive to be called so 
frequently. So, in current implementation, I use a timer to periodically update 
the histograms. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1007] Added tuple arrival rate and tupl...

2015-09-05 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/716#issuecomment-137968137
  
Hi @HeartSaVioR,

Thank you for your comment. I understand your concern, as 
```DisruptorQueue``` plays an important role in Storm.

Following your suggestion, I have done performance test using 
[https://github.com/yahoo/storm-perf-test]. As expected, the results show that 
the new metrics do not introduce noticeable overhead.

Environment:


  Factors | Configurations
 | -
CPU | Intel(R) Xeon(R) CPU E5-1620 v3 @ 3.50GHz
RAM | 16GB
DISK | 2TB
OS | Ubuntu 14.02


Parameters:

```
--bolt 3 --name test -l 1 -n 1 --workers 4 --spout 3 --testTimeSec 900 -c 
topology.max.spout.pending=1092 --messageSize 10
```


Result summary:

master at 154e9ec55deb4eea8fca8554e4d3b224bf337834:
Throughput in MB/s
AVG: 7.974807415
MIN : 7.878930664
MAX: 8.095523041

PR at f15ac94d259a96ac891f5243af29b0d953ea863d:
Throughput in MB/s
AVG: 7.977237486
MIN : 7.844562531
MAX: 8.251005809

===

The details of runs at master(154e9ec55deb4eea8fca8554e4d3b224bf337834)
```
RUNNING 1   4   4   6   6   1441452581500   3   
244704007.878930664
RUNNING 1   4   4   6   6   1441452611500   3   
249626807.935422262
RUNNING 1   4   4   6   6   1441452641500   3   
247659607.972886658
RUNNING 1   4   4   6   6   1441452671500   3   
245766207.982697093
RUNNING 1   4   4   6   6   1441452701500   3   
249018607.916088104
RUNNING 1   4   4   6   6   1441452731500   3   
251910408.008015951
RUNNING 1   4   4   6   6   1441452761501   30001   
247911807.880641192
RUNNING 1   4   4   6   6   1441452791500   2   
251268007.987860867
RUNNING 1   4   4   6   6   1441452821500   3   
250711807.969913483
RUNNING 1   4   4   6   6   1441452851500   3   
250985407.978610992
RUNNING 1   4   4   6   6   1441452881501   30001   
249661207.996251266
RUNNING 1   4   4   6   6   1441452911500   2   
248363408.095523041
RUNNING 1   4   4   6   6   1441452941500   3   
250044807.968710124
RUNNING 1   4   4   6   6   1441452971500   3   
253713608.065338135
RUNNING 1   4   4   6   6   1441453001500   3   
249721607.938435872
RUNNING 1   4   4   6   6   1441453031500   3   
253820208.068726858
RUNNING 1   4   4   6   6   1441453061500   3   
250212807.9540507
RUNNING 1   4   4   6   6   1441453091500   3   
250111207.950820923
RUNNING 1   4   4   6   6   1441453121500   3   
246906807.94895579
RUNNING 1   4   4   6   6   1441453151500   3   
249540607.972682037
RUNNING 1   4   4   6   6   1441453181500   3   
248990407.98519165
RUNNING 1   4   4   6   6   1441453211501   30001   
247374607.963564633
RUNNING 1   4   4   6   6   1441453241500   2   
249960607.946298354
RUNNING 1   4   4   6   6   1441453271500   3   
249861007.942867279
RUNNING 1   4   4   6   6   1441453301500   3   
249069207.997696635
RUNNING 1   4   4   6   6   1441453331500   3   
250415207.960484823
RUNNING 1   4   4   6   6   1441453361500   3   
251218807.986030579
RUNNING 1   4   4   6   6   1441453391500   3   
251263807.98746109
RUNNING 1   4   4   6   6   1441453421500   3   
250438607.981228689
RUNNING 1   4   4   6   6   1441453451501   30001   
249239208.022836695
RUNNING 1   4   4   6   6   1441448928080   3   
243575807.974807415
RUNNING 1   4   4   6   6   1441448958080   3   
244973607.878930664
```

The details of runs at this PR(f15ac94d259a96ac891f5243af29b0d953ea863d)
```
RUNNING 1   4   4   6   6   1441450879792   30001   
25703700

[GitHub] storm pull request: [STORM-1007] Added tuple arrival rate and tupl...

2015-09-03 Thread wangli1426 
GitHub user wangli1426 opened a pull request:

https://github.com/apache/storm/pull/716

[STORM-1007] Added tuple arrival rate and tuple sojourn time to QueueMetrics

I added two important metrics, namely *arrival time* and *sojourn time*, to 
QueueMetrics. 

Modification Summary:
1. Created a new RateTracker utility, which could report instantaneous rate 
upon certain event;
2. Added test cases for RateTracker;
3. Added arrival rate and sojourn time metrics to QueueMetrics; 
4. Use sampling in QueueMetrics to reduce the metric maintenance cost. 

Such metrics are very useful to performance tuning, especially for 
application with strong constraint on processing latency such as thread 
detection and smart traffic. A high sojourn time, for example, indicates that a 
higher degree of parallelism should be applied to the current bolt in order to 
reduce processing delay. 

Any comment or suggestion regarding to this pull request is welcomed. 

Thanks.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/wangli1426/storm queue-metrics

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/716.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 #716


commit 3d0e79e56089023a0b731fc2f4f66f24cd3264e1
Author: wangli1426 <wangli1...@gmail.com>
Date:   2015-08-26T09:40:34Z

Added arrival_rate metric to DisruptorQueue.

commit 9cd94bb865c527fb8f5c6728670f97cb9073e0f8
Author: Li Wang <wangli1...@gmail.com>
Date:   2015-09-03T07:57:15Z

Added Tests for RateTracker

commit fbf85db2f152440fffebd432f3106ddfdf1a848d
Author: Li Wang <wangli1...@gmail.com>
Date:   2015-09-03T07:58:24Z

Added sampling feature to QueueMetrics to reduce the costs.

commit d026c49c594fdcbbb2665c116b1446798400b0bf
Author: wangli1426 <wangli1...@gmail.com>
Date:   2015-09-03T11:51:51Z

Added element sojourn time, a new metric, to QueueMetrics.




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1008] Isolate the code for metric colle...

2015-08-27 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/698#issuecomment-135603939
  
Hi @HeartSaVioR,
Thanks for the comment. I'm sorry that It should be STORM-1008. I made a 
mistake on the number. And I have corrected the title of this PR. Please 
consider merging it or give me some advances. I will appreciate it. Thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1007] Isolate the code for metric colle...

2015-08-26 Thread wangli1426 
GitHub user wangli1426 opened a pull request:

https://github.com/apache/storm/pull/698

[STORM-1007] Isolate the code for metric collection and retrieval from 
DisruptorQueue

I created an inner class QueueMetrics for DisruptorQueue. The QueueMetrics 
class handles metric collection logics and exposes metric retrieval methods.

The benefits of such modification are twofold: 
1. It improves the readability of the metric code.
2. It facilitates to add more metrics, such as average throughput and 
element sojourn time.  (That is because the data  maintained for such 
stateful metrics can reside in QueueMetrics rather than in DisruptorQueue. )

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/wangli1426/storm master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/698.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 #698


commit 4337ebe6e2398073e9a238e88bd51ca7b76b3437
Author: wangli1426 wangli1...@gmail.com
Date:   2015-08-26T04:48:27Z

Created QueueMetrics inner class in DistruptorQueue to improve the 
extendibility and the readability of the metrics 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 this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-992] A bug in the timer.clj might cause...

2015-08-13 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/680#issuecomment-130939524
  
@knusbaum I have changed the title of this pull request as suggested. I am 
sorry for your inconvenience. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: Update timer.clj

2015-08-13 Thread wangli1426 
Github user wangli1426 commented on the pull request:

https://github.com/apache/storm/pull/680#issuecomment-130932282
  
@knusbaum I have filed a jira at 
https://issues.apache.org/jira/browse/STORM-992.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: Update timer.clj

2015-08-12 Thread wangli1426 
Github user wangli1426 commented on a diff in the pull request:

https://github.com/apache/storm/pull/680#discussion_r36938783
  
--- Diff: storm-core/src/clj/backtype/storm/timer.clj ---
@@ -53,8 +53,11 @@
;; event generation. If any recurring 
events
;; are scheduled then we will always go
;; through this branch, sleeping only 
the
-   ;; exact necessary amount of time.
-   (Time/sleep (- time-millis 
(current-time-millis)))
+   ;; exact necessary amount of time. We 
give
+   ;; an upper bound, e.g. 1000 millis, to 
the
+   ;; sleeping time, to limit the response 
time
+   ;; for detecting any new event within 1 
secs.
+   (Time/sleep (max 1000 (- time-millis 
(current-time-millis
--- End diff --

Yes, you are right, min should be used definitely. Thanks


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: Update timer.clj

2015-08-12 Thread wangli1426 
GitHub user wangli1426 opened a pull request:

https://github.com/apache/storm/pull/680

Update timer.clj

Issue description: The timer thread calculates the delay to schedule the 
head of the queue and sleeps accordingly. However, if a new event with high 
priority is inserted at the head of the queue during the sleep, this event 
cannot be scheduled in time. 

Solution: Give a upper bound to the sleeping time, to guarantee a bounded 
response time for detecting new events.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/wangli1426/storm master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/680.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 #680


commit d1a6f4a753661f7e825eeec0c51782462fca4d5f
Author: Li Wang wangli1...@gmail.com
Date:   2015-08-12T14:00:15Z

Update timer.clj

Give a upper bound to the sleeping time, to guarantee a bounded response 
time for detecting new events.




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---