[GitHub] storm pull request: STORM-1698 Asynchronous MetricsConsumerBolt (1...

[HeartSaVioR]

Github user HeartSaVioR closed the pull request at:

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


---
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-1698 Asynchronous MetricsConsumerBolt

[HeartSaVioR]

Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/1322#issuecomment-216879711
  
Will be also addressed in #1324, closing.


---
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-1698 Asynchronous MetricsConsumerBolt

[HeartSaVioR]

Github user HeartSaVioR closed the pull request at:

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


---
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-1698 Asynchronous MetricsConsumerBolt (1...

[HeartSaVioR]

Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/1323#issuecomment-216879633
  
Will be also addressed in #1325, closing.


---
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-1698 Asynchronous MetricsConsumerBolt

[unsleepy22]

Github user unsleepy22 commented on the pull request:

https://github.com/apache/storm/pull/1322#issuecomment-213352632
  
I suppose this PR has been merged into 
https://github.com/apache/storm/pull/1324 ? I'll skip this one.


---
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-1698 Asynchronous MetricsConsumerBolt

[HeartSaVioR]

Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/1322#issuecomment-207717343
  
I'm addressing the (2), and being wondering that we have to address (1) 
since it drops metrics data anyway so it should be avoided.


---
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-1698 Asynchronous MetricsConsumerBolt

[HeartSaVioR]

Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/1322#issuecomment-207321201
  
Storm already use codahale metric registery but it's used for exposing 
internal metrics via JXM.


---
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-1698 Asynchronous MetricsConsumerBolt

[HeartSaVioR]

Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/1322#issuecomment-207319131
  
You got me. :)
I was also finding aggregation metrics on worker side, but after struggling 
I gave it. 
Why I gave is that IMetric.getValueAndReset() is not multi-threads 
friendly. Currently metrics are updated at receive queue handler thread of 
executors, and that thread also handles metrics tick tuple so there's no race 
condition. 
Maybe most way of in process aggregation should resolve this issue.



---
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-1698 Asynchronous MetricsConsumerBolt

[HeartSaVioR]

Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/1322#issuecomment-207319487
  
gave it -> gave up


---
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-1698 Asynchronous MetricsConsumerBolt

[abhishekagarwal87]

Github user abhishekagarwal87 commented on the pull request:

https://github.com/apache/storm/pull/1322#issuecomment-207310576
  
Filtering would be a good feature to add. I have also been trying to gather 
some opinion on in-process aggregation at component level, without much success 
:) Codahale metric registry is a great alternative but that would translating 
many metrics in the storm-core to codahale compliant implementations e.g. 
CountMetric to Counter. I am willing to spend that effort if approach sounds 
good. 


---
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-1698 Asynchronous MetricsConsumerBolt

[HeartSaVioR]

Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/1322#issuecomment-207251473
  
@abhishekagarwal87 
Yes, I also have two ideas to avoid or minimize those issues.

1) discard policy
2) filter metrics

About 1) _taskQueue will become bounded queue. And when it's full, we 
discard some enqueued tasks via policy. Policy could be drop oldest, drop 
latest, drop randomly, or even provide interface for plug-in.

About 2), actually there're many metrics provided by Storm by default. 
Furthermore, some modules like storm-kafka provides their metrics, too. If 
users set pattern what they want to subscribe or filter out that would be great.

I'm going to create an umbrella issue regarding improvement of metrics and 
add STORM-1698 as subtask.


---
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-1698 Asynchronous MetricsConsumerBolt

[abhishekagarwal87]

Github user abhishekagarwal87 commented on the pull request:

https://github.com/apache/storm/pull/1322#issuecomment-207242185
  
The _taskQueue is unbounded queue and if metric consumer bolt is indeed 
slow, this queue would keep growing, leading to another set of problems. Though 
it is probably ok as a short term fix. 


---
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-1698 Asynchronous MetricsConsumerBolt

[HeartSaVioR]

GitHub user HeartSaVioR opened a pull request:

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

STORM-1698 Asynchronous MetricsConsumerBolt

* change MetricsConsumerBolt's behavior to asynchronus manner
  * to avoid bad side effect of topology
* for details please refer JIRA issue: 
https://issues.apache.org/jira/browse/STORM-1698

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

$ git pull https://github.com/HeartSaVioR/storm STORM-1698

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

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


commit e1db0db86eeb3857bc6f958ef0af81d06ff0d9af
Author: Jungtaek Lim 
Date:   2016-04-08T04:05:00Z

STORM-1698 Asynchronous MetricsConsumerBolt

* change MetricsConsumerBolt's behavior to asynchronus manner
  * to avoid bad side effect of topology
* for details please refer JIRA issue: 
https://issues.apache.org/jira/browse/STORM-1698




---
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.
---