steveloughran commented on code in PR #4092:
URL: https://github.com/apache/hadoop/pull/4092#discussion_r851626510
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java:
##########
@@ -430,10 +447,13 @@ protected synchronized byte[] createPassword(TokenIdent
identifier) {
DelegationTokenInformation tokenInfo = new DelegationTokenInformation(now
+ tokenRenewInterval, password, getTrackingIdIfEnabled(identifier));
try {
+ long start = Time.monotonicNow();
Review Comment:
`IOStatisticsStore` is a `DurationTrackerFactory`; you can use it in
IOStatisticsBinding invocations to have it track min/mean/max duration of ops.
distinguishing success and failure times
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java:
##########
@@ -96,6 +108,10 @@ private String formatTokenId(TokenIdent id) {
* Access to currentKey is protected by this object lock
*/
private DelegationKey currentKey;
+ /**
+ * Metrics to track token management operations
Review Comment:
nit: add a trailing .
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java:
##########
@@ -825,4 +859,68 @@ protected void syncTokenOwnerStats() {
addTokenForOwnerStats(id);
}
}
+
+ /**
+ * DelegationTokenSecretManagerMetrics tracks token management operations
+ * and publishes them through the metrics interfaces.
+ */
+ @Metrics(about="Delegation token secret manager metrics", context="token")
+ static class DelegationTokenSecretManagerMetrics implements
IOStatisticsSource {
Review Comment:
make this a DurationTrackerFactory and you can supply duration trackers to
IOStatisticsBinding; note also there's a `PairedDurationTrackerFactory` which
you could wire up the iostats store. maybe this would be the time/place to add
a DurationTrackerFactory for updating hadoop metrics
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestDelegationToken.java:
##########
@@ -579,4 +616,55 @@ public void testEmptyToken() throws IOException {
assertEquals(token1, token2);
assertEquals(token1.encodeToUrlString(), token2.encodeToUrlString());
}
+
+ @Test
+ public void testDelegationTokenSecretManagerMetrics() throws Exception {
+ TestDelegationTokenSecretManager dtSecretManager =
+ new TestDelegationTokenSecretManager(24*60*60*1000,
+ 10*1000, 1*1000, 60*60*1000);
+ try {
+ dtSecretManager.startThreads();
+
+ Assert.assertEquals(0,
dtSecretManager.metrics.storeToken.lastStat().numSamples());
Review Comment:
this is invoked enough it is worth factoring into its own assertion
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestDelegationToken.java:
##########
@@ -579,4 +616,55 @@ public void testEmptyToken() throws IOException {
assertEquals(token1, token2);
assertEquals(token1.encodeToUrlString(), token2.encodeToUrlString());
}
+
+ @Test
+ public void testDelegationTokenSecretManagerMetrics() throws Exception {
+ TestDelegationTokenSecretManager dtSecretManager =
+ new TestDelegationTokenSecretManager(24*60*60*1000,
+ 10*1000, 1*1000, 60*60*1000);
+ try {
+ dtSecretManager.startThreads();
+
+ Assert.assertEquals(0,
dtSecretManager.metrics.storeToken.lastStat().numSamples());
+ final Token<TestDelegationTokenIdentifier> token =
+ generateDelegationToken(dtSecretManager, "SomeUser", "JobTracker");
+ Assert.assertEquals(1,
dtSecretManager.metrics.storeToken.lastStat().numSamples());
+
+ Assert.assertEquals(0,
dtSecretManager.metrics.updateToken.lastStat().numSamples());
+ dtSecretManager.renewToken(token, "JobTracker");
+ Assert.assertEquals(1,
dtSecretManager.metrics.updateToken.lastStat().numSamples());
+
+ Assert.assertEquals(0,
dtSecretManager.metrics.removeToken.lastStat().numSamples());
+ dtSecretManager.cancelToken(token, "JobTracker");
+ Assert.assertEquals(1,
dtSecretManager.metrics.removeToken.lastStat().numSamples());
+ } finally {
+ dtSecretManager.stopThreads();
+ }
+ }
+
+ @Test
+ public void testDelegationTokenSecretManagerMetricsFailures() throws
Exception {
+ TestFailureDelegationTokenSecretManager dtSecretManager = new
TestFailureDelegationTokenSecretManager();
+
+ try {
+ dtSecretManager.startThreads();
+
+ final Token<TestDelegationTokenIdentifier> token =
+ generateDelegationToken(dtSecretManager, "SomeUser", "JobTracker");
+
+ dtSecretManager.setThrowError(true);
+
+ Assert.assertEquals(0, dtSecretManager.metrics.tokenFailure.value());
+ generateDelegationToken(dtSecretManager, "SomeUser", "JobTracker");
+ Assert.assertEquals(1, dtSecretManager.metrics.tokenFailure.value());
+
+ LambdaTestUtils.intercept(Exception.class, () ->
dtSecretManager.renewToken(token, "JobTracker"));
Review Comment:
also, if on failure that renew call sleeps for a few hundred millis, you
could assert that the duration of the failure was measured/counted.
IOStatisticAssertions helps there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
