-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-17081. MetricsSystem doesn't start the sink adapters on restart #2089
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
Conversation
|
💔 -1 overall
This message was automatically generated. |
d759021 to
eeed85b
Compare
What changed - The sink registration starts the adapter if it doesn't exists.
| try { | ||
| ms.start(); | ||
| ms.register(sinkName, "", ts); | ||
| assertNotNull("an adapter should exist for each sink", ms.getSinkAdapter(sinkName)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're generally still 80 chars wide, I'm afraid...put the actual probe on a new line.
nice to see some text for the assertion tnough -appreciated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it, please review (added more info to the message)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, +1 pending that checkstyle fixup. [Don't worry about yetus reporting javadoc issues on java 11 BTW]
|
💔 -1 overall
This message was automatically generated. |
|
+1 merged to trunk and about to pull into branch-3.3 |
#2089) Contributed by Madhusoodan P
#2089) Contributed by Madhusoodan P
|
and 3.2 BTW, if you are looking at metrics, why not look at #2069 . That's not about publishing externally, but it is designed to enable apps to collect statistics in detail about an instance of an IO object (stream etc) for collation |
What changed