[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-11-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2203 The difference on CPU usage and latency is reduced again, which looks like closer. (We need to be aware that test results from same branch fluctuate a bit.) ---

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-11-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2203 I created another branch for sampling: https://github.com/HeartSaVioR/storm/tree/metrics_v2_replace_meters_to_counters_with_sampling > metrics_v2_replace_meters_to_counters_with_sampl

[GitHub] storm pull request #2433: [STORM-2693] Heartbeats and assignments promotion ...

2017-11-27 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/2433#discussion_r153401446 --- Diff: storm-client/src/jvm/org/apache/storm/stats/StatsUtil.java --- @@ -1589,18 +1570,16 @@ public static ComponentPageInfo aggCompExecsStats(

[GitHub] storm issue #2365: [STORM-2773]If a drpcserver node in cluster is down,drpc ...

2017-11-27 Thread liu-zhaokun
Github user liu-zhaokun commented on the issue: https://github.com/apache/storm/pull/2365 @HeartSaVioR Hi,are you available to help me review this PR? ---

[GitHub] storm issue #2430: [STORM-2827] fix Logviewer search returning incorrect log...

2017-11-27 Thread Ethanlm
Github user Ethanlm commented on the issue: https://github.com/apache/storm/pull/2430 @srdo Thanks for the review. @revans2 @HeartSaVioR Could you please take a second look? Thanks ---

[GitHub] storm issue #2436: STORM-2833: Use the original host name when closing (2.x)

2017-11-27 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2436 Thanks, still +1 ---

[GitHub] storm issue #2436: STORM-2833: Use the original host name when closing (2.x)

2017-11-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2436 @srdo I made the name change you suggested. ---

[GitHub] storm pull request #2436: STORM-2833: Use the original host name when closin...

2017-11-27 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/2436#discussion_r153303198 --- Diff: storm-client/src/jvm/org/apache/storm/messaging/netty/Client.java --- @@ -82,6 +82,10 @@ private final ClientBootstrap bootstrap; pr

[GitHub] storm issue #2436: STORM-2833: Use the original host name when closing (2.x)

2017-11-27 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2436 +1 ---

[GitHub] storm pull request #2436: STORM-2833: Use the original host name when closin...

2017-11-27 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2436 STORM-2833: Use the original host name when closing. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2833 Alterna

[GitHub] storm pull request #2435: STORM-2833: use the same host name for removal too

2017-11-27 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2435 STORM-2833: use the same host name for removal too You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2833-1.x Alter

[GitHub] storm issue #2430: [STORM-2827] fix Logviewer search returning incorrect log...

2017-11-27 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2430 +1, thanks for filing the followup issue too. ---

[GitHub] storm issue #2430: [STORM-2827] fix Logviewer search returning incorrect log...

2017-11-27 Thread Ethanlm
Github user Ethanlm commented on the issue: https://github.com/apache/storm/pull/2430 I filed a separate jira for readStormConfig() related issue because it looks like there are many places using wrong function. We need to identify them and change them. JIRA: https://issues.apache

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-11-27 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2203 I did a quick analysis of the latency vs CPU usage for the branch from @HeartSaVioR and this branch based off of the test results posted by @HeartSaVioR (just to get more of an apples to apples compa

[GitHub] storm pull request #2430: [STORM-2827] fix Logviewer search returning incorr...

2017-11-27 Thread Ethanlm
Github user Ethanlm commented on a diff in the pull request: https://github.com/apache/storm/pull/2430#discussion_r15328 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogSearchHandler.java --- @@ -97,8 +98,14 @@ public LogviewerLogSear

[GitHub] storm pull request #2430: [STORM-2827] fix Logviewer search returning incorr...

2017-11-27 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/2430#discussion_r153232875 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogSearchHandler.java --- @@ -97,8 +98,14 @@ public LogviewerLogSearchH

[GitHub] storm pull request #2430: [STORM-2827] fix Logviewer search returning incorr...

2017-11-27 Thread Ethanlm
Github user Ethanlm commented on a diff in the pull request: https://github.com/apache/storm/pull/2430#discussion_r153216647 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogSearchHandler.java --- @@ -97,8 +98,14 @@ public LogviewerLogSear

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-11-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2203 So the difference on CPU usage and latency is reduced, whereas there's still gap between twos. That's what we can imagine, so please do some performance tests and see the gaps, and discuss whethe

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-11-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2203 > metrics_v2_replace_meters_to_counters ``` uptime: 210 acked: 1,504,280 acked/sec: 50,142.67 failed:0 99%: 35,192,831 99.9%: 52,658,175 min: 6,561,792 max:

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-11-27 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2203 I've tried out replacing meters to counters: here is the branch https://github.com/HeartSaVioR/storm/tree/metrics_v2_replace_meters_to_counters ---