[GitHub] storm issue #2764: STORM-3147: Port ClusterSummary as metrics to StormMetric...
Github user zd-project commented on the issue: https://github.com/apache/storm/pull/2764 cachedSummary shouldn't be refreshed too often based on current implementation. So I don't think we should risk the race condition just for negligible gain in performance. ---
[GitHub] storm issue #2764: STORM-3147: Port ClusterSummary as metrics to StormMetric...
Github user zd-project commented on the issue: https://github.com/apache/storm/pull/2764 What do you think @srdo ---
[GitHub] storm issue #2764: STORM-3147: Port ClusterSummary as metrics to StormMetric...
Github user zd-project commented on the issue: https://github.com/apache/storm/pull/2764 Added clarifications to ClusterSummaryMetricSet ---
[GitHub] storm issue #2764: STORM-3147: Port ClusterSummary as metrics to StormMetric...
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2764 I'd like https://github.com/apache/storm/pull/2754 to go in first, so we're sure the review from there is reflected here. When that one is merged, I'd like to take another look at these changes. ---
[GitHub] storm issue #2764: STORM-3147: Port ClusterSummary as metrics to StormMetric...
Github user zd-project commented on the issue: https://github.com/apache/storm/pull/2764 All nits addressed? ---
[GitHub] storm issue #2764: STORM-3147: Port ClusterSummary as metrics to StormMetric...
Github user zd-project commented on the issue: https://github.com/apache/storm/pull/2764 Please notice that only the last commit is under #2764, which was previously out of sync with #2754 because I don't want to have too many dependencies between PR. For issue specific to #2754 I think it's better to review it there. @srdo ---
[GitHub] storm issue #2764: STORM-3147: Port ClusterSummary as metrics to StormMetric...
Github user zd-project commented on the issue: https://github.com/apache/storm/pull/2764 Has been rebased on 3133 now. ---
[GitHub] storm issue #2764: STORM-3147: Port ClusterSummary as metrics to StormMetric...
Github user zd-project commented on the issue: https://github.com/apache/storm/pull/2764 This can be merged after #2789 ---
