From 1e043b937a0652b45eb5f1314e96aa754f482c50 Mon Sep 17 00:00:00 2001 From: hchaverr Date: Thu, 5 May 2022 12:39:58 -0700 Subject: [PATCH] HADOOP-18222. Prevent DelegationTokenSecretManagerMetrics from registering multiple times Fixes #4266 Signed-off-by: Owen O'Malley --- .../AbstractDelegationTokenSecretManager.java | 19 +++---- .../token/delegation/TestDelegationToken.java | 52 +++++++++++++------ 2 files changed, 45 insertions(+), 26 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java index d348d6a79a..89ac6846fc 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java @@ -65,6 +65,12 @@ class AbstractDelegationTokenSecretManager storeToken(identifier, tokenInfo)); + METRICS.trackStoreToken(() -> storeToken(identifier, tokenInfo)); } catch (IOException ioe) { LOG.error("Could not store token " + formatTokenId(identifier) + "!!", ioe); @@ -558,7 +559,7 @@ public synchronized long renewToken(Token token, throw new InvalidToken("Renewal request for unknown token " + formatTokenId(id)); } - metrics.trackUpdateToken(() -> updateToken(id, info)); + METRICS.trackUpdateToken(() -> updateToken(id, info)); return renewTime; } @@ -594,7 +595,7 @@ public synchronized TokenIdent cancelToken(Token token, if (info == null) { throw new InvalidToken("Token not found " + formatTokenId(id)); } - metrics.trackRemoveToken(() -> { + METRICS.trackRemoveToken(() -> { removeStoredToken(id); }); return id; @@ -745,7 +746,7 @@ public TokenIdent decodeTokenIdentifier(Token token) throws IOExcept } protected DelegationTokenSecretManagerMetrics getMetrics() { - return metrics; + return METRICS; } /** diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestDelegationToken.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestDelegationToken.java index ca0b18bb1d..225cc658d3 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestDelegationToken.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestDelegationToken.java @@ -33,6 +33,7 @@ import java.util.concurrent.Callable; import org.apache.hadoop.fs.statistics.IOStatisticAssertions; import org.apache.hadoop.fs.statistics.MeanStatistic; +import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; import org.apache.hadoop.metrics2.lib.MutableCounterLong; import org.apache.hadoop.metrics2.lib.MutableRate; import org.apache.hadoop.test.LambdaTestUtils; @@ -635,6 +636,23 @@ public void testEmptyToken() throws IOException { assertEquals(token1.encodeToUrlString(), token2.encodeToUrlString()); } + @Test + public void testMultipleDelegationTokenSecretManagerMetrics() { + TestDelegationTokenSecretManager dtSecretManager1 = + new TestDelegationTokenSecretManager(0, 0, 0, 0); + assertNotNull(dtSecretManager1.getMetrics()); + + TestDelegationTokenSecretManager dtSecretManager2 = + new TestDelegationTokenSecretManager(0, 0, 0, 0); + assertNotNull(dtSecretManager2.getMetrics()); + + DefaultMetricsSystem.instance().init("test"); + + TestDelegationTokenSecretManager dtSecretManager3 = + new TestDelegationTokenSecretManager(0, 0, 0, 0); + assertNotNull(dtSecretManager3.getMetrics()); + } + @Test public void testDelegationTokenSecretManagerMetrics() throws Exception { TestDelegationTokenSecretManager dtSecretManager = @@ -645,13 +663,13 @@ public void testDelegationTokenSecretManagerMetrics() throws Exception { final Token token = callAndValidateMetrics( dtSecretManager, dtSecretManager.getMetrics().getStoreToken(), "storeToken", - () -> generateDelegationToken(dtSecretManager, "SomeUser", "JobTracker"), 1); + () -> generateDelegationToken(dtSecretManager, "SomeUser", "JobTracker")); callAndValidateMetrics(dtSecretManager, dtSecretManager.getMetrics().getUpdateToken(), - "updateToken", () -> dtSecretManager.renewToken(token, "JobTracker"), 1); + "updateToken", () -> dtSecretManager.renewToken(token, "JobTracker")); callAndValidateMetrics(dtSecretManager, dtSecretManager.getMetrics().getRemoveToken(), - "removeToken", () -> dtSecretManager.cancelToken(token, "JobTracker"), 1); + "removeToken", () -> dtSecretManager.cancelToken(token, "JobTracker")); } finally { dtSecretManager.stopThreads(); } @@ -671,14 +689,14 @@ public void testDelegationTokenSecretManagerMetricsFailures() throws Exception { dtSecretManager.setThrowError(true); - callAndValidateFailureMetrics(dtSecretManager, "storeToken", 1, 1, false, + callAndValidateFailureMetrics(dtSecretManager, "storeToken", false, errorSleepMillis, () -> generateDelegationToken(dtSecretManager, "SomeUser", "JobTracker")); - callAndValidateFailureMetrics(dtSecretManager, "updateToken", 1, 2, true, + callAndValidateFailureMetrics(dtSecretManager, "updateToken", true, errorSleepMillis, () -> dtSecretManager.renewToken(token, "JobTracker")); - callAndValidateFailureMetrics(dtSecretManager, "removeToken", 1, 3, true, + callAndValidateFailureMetrics(dtSecretManager, "removeToken", true, errorSleepMillis, () -> dtSecretManager.cancelToken(token, "JobTracker")); } finally { dtSecretManager.stopThreads(); @@ -686,33 +704,33 @@ public void testDelegationTokenSecretManagerMetricsFailures() throws Exception { } private T callAndValidateMetrics(TestDelegationTokenSecretManager dtSecretManager, - MutableRate metric, String statName, Callable callable, int expectedCount) + MutableRate metric, String statName, Callable callable) throws Exception { MeanStatistic stat = IOStatisticAssertions.lookupMeanStatistic( dtSecretManager.getMetrics().getIoStatistics(), statName + ".mean"); - assertEquals(expectedCount - 1, metric.lastStat().numSamples()); - assertEquals(expectedCount - 1, stat.getSamples()); + long metricBefore = metric.lastStat().numSamples(); + long statBefore = stat.getSamples(); T returnedObject = callable.call(); - assertEquals(expectedCount, metric.lastStat().numSamples()); - assertEquals(expectedCount, stat.getSamples()); + assertEquals(metricBefore + 1, metric.lastStat().numSamples()); + assertEquals(statBefore + 1, stat.getSamples()); return returnedObject; } private void callAndValidateFailureMetrics(TestDelegationTokenSecretManager dtSecretManager, - String statName, int expectedStatCount, int expectedMetricCount, boolean expectError, - int errorSleepMillis, Callable callable) throws Exception { + String statName, boolean expectError, int errorSleepMillis, Callable callable) + throws Exception { MutableCounterLong counter = dtSecretManager.getMetrics().getTokenFailure(); MeanStatistic failureStat = IOStatisticAssertions.lookupMeanStatistic( dtSecretManager.getMetrics().getIoStatistics(), statName + ".failures.mean"); - assertEquals(expectedMetricCount - 1, counter.value()); - assertEquals(expectedStatCount - 1, failureStat.getSamples()); + long counterBefore = counter.value(); + long statBefore = failureStat.getSamples(); if (expectError) { LambdaTestUtils.intercept(IOException.class, callable); } else { callable.call(); } - assertEquals(expectedMetricCount, counter.value()); - assertEquals(expectedStatCount, failureStat.getSamples()); + assertEquals(counterBefore + 1, counter.value()); + assertEquals(statBefore + 1, failureStat.getSamples()); assertTrue(failureStat.getSum() >= errorSleepMillis); } }