Skip to content

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.

Sign up for GitHub

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

Merged
steveloughran merged 3 commits into from
Jul 6, 2020
Merged

HADOOP-17081. MetricsSystem doesn't start the sink adapters on restart #2089

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e670b34
HADOOP-17081 : MetricsSystem doesn't start the sink adapters on restart
mmpataki Jun 22, 2020
572df34
Stop the metric system post testing
mmpataki Jun 23, 2020
6dc79df
Fixed the 80char checkstyle error
mmpataki Jul 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ...roject/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,11 @@ void registerSource(String name, String desc, MetricsSource source) {
T register(final String name, final String description, final T sink) {
LOG.debug(name +", "+ description);
if (allSinks.containsKey(name)) {
LOG.warn("Sink "+ name +" already exists!");
if(sinks.get(name) == null) {
registerSink(name, description, sink);
} else {
LOG.warn("Sink "+ name +" already exists!");
}
return sink;
}
allSinks.put(name, sink);
Expand Down
19 changes: 19 additions & 0 deletions ...ct/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestMetricsSystemImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,4 +639,23 @@ private static class TestSource2 {
private static String getPluginUrlsAsString() {
return "file:metrics2-test-plugin.jar";
}

@Test
public void testMetricSystemRestart() {
MetricsSystemImpl ms = new MetricsSystemImpl("msRestartTestSystem");
TestSink ts = new TestSink();
String sinkName = "restartTestSink";

try {
ms.start();
ms.register(sinkName, "", ts);
assertNotNull("an adapter should exist for each sink", ms.getSinkAdapter(sinkName));
Copy link
Contributor

@steveloughran steveloughran Jul 3, 2020

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

Copy link
Contributor Author

@mmpataki mmpataki Jul 4, 2020

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)

ms.stop();

ms.start();
assertNotNull("an adapter should exist for each sink", ms.getSinkAdapter(sinkName));
} finally {
ms.stop();
}
}
}