Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STORM-2909 port new metrics to 2.x branch #2624

Merged
merged 1 commit into from
Apr 10, 2018

Conversation

agresch
Copy link
Contributor

@agresch agresch commented Apr 6, 2018

@ptgoetz

Ported the new metrics code from 1.2 to 2.x. Verified ConsoleReporter spit out some of the new metrics.

Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I compared this with #2203 by side to side, though this contains huge lines of additional style fixes. Left minor comments.

@@ -223,7 +224,7 @@ private Object loadWorker(Map<String, Object> topologyConf, IStateStorage stateS


// This thread will send out messages destined for remote tasks (on other workers)
if ( ( (Long)topologyConf.get(Config.TOPOLOGY_WORKERS) ) > 1 ) {
if (((Long)topologyConf.get(Config.TOPOLOGY_WORKERS)) > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase with latest master.


@Override
public void prepare(Map<String, Object> StormConf){
public void prepare(Map stormConf){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep exposing generic type if possible.

@HeartSaVioR
Copy link
Contributor

Could you take up copying documentation from another PR as well, based on #2526?

@agresch
Copy link
Contributor Author

agresch commented Apr 9, 2018

@HeartSaVioR made the changes you suggested. I can look at the documentation changes.

Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@asfgit asfgit merged commit 21da5df into apache:master Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants