szetszwo commented on code in PR #8888:
URL: https://github.com/apache/hadoop/pull/8888#discussion_r4418350830


##########
dev-support/benchmarks/kms-http-benchmark.py:
##########
@@ -0,0 +1,160 @@
+#!/usr/bin/env python3

Review Comment:
   Let's move the benchmark and readme to ` 
hadoop-common-project/hadoop-common/src/test/scripts/kms-http-benchmark`.
   



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java:
##########
@@ -561,31 +561,49 @@ private void removeExpiredKeys() {
   protected byte[] createPassword(TokenIdent identifier) {
     this.apiLock.writeLock().lock();
     try {
-      int sequenceNum;
-      long now = Time.now();
-      sequenceNum = incrementDelegationTokenSeqNum();
-      identifier.setIssueDate(now);
-      identifier.setMaxDate(now + tokenMaxLifetime);
-      identifier.setMasterKeyId(currentKey.getKeyId());
-      identifier.setSequenceNumber(sequenceNum);
-      LOG.info("Creating password for identifier: " + 
formatTokenId(identifier) +
-          ", currentKey: " + currentKey.getKeyId());
-      byte[] password = createPassword(identifier.getBytes(), 
currentKey.getKey());
-      DelegationTokenInformation tokenInfo =
-          new DelegationTokenInformation(now + tokenRenewInterval, password,
-              getTrackingIdIfEnabled(identifier));
+      DelegationTokenInformation tokenInfo = prepareToken(identifier,
+          incrementDelegationTokenSeqNum());
       try {
         METRICS.trackStoreToken(() -> storeToken(identifier, tokenInfo));
       } catch (IOException ioe) {
         LOG.error("Could not store token " + formatTokenId(identifier) + "!!", 
ioe);
       }
-      return password;
+      return tokenInfo.getPassword();
     } finally {
       this.apiLock.writeLock().unlock();
     }
   }
 
-
+  /**
+   * Initialize an identifier and its password using a consistent signing key.
+   * Sequence allocation and persistence are the caller's responsibility and
+   * must not be performed inside this critical section by external stores.
+   * The caller must not modify the identifier after preparation.
+   *
+   * @param identifier identifier to initialize
+   * @param sequenceNumber previously allocated unique sequence number
+   * @return prepared token information
+   */
+  protected final DelegationTokenInformation prepareToken(TokenIdent 
identifier,
+      int sequenceNumber) {
+    DelegationTokenInformation tokenInfo;
+    this.apiLock.writeLock().lock();
+    try {
+      long now = Time.now();
+      identifier.setIssueDate(now);
+      identifier.setMaxDate(now + tokenMaxLifetime);
+      identifier.setMasterKeyId(currentKey.getKeyId());
+      identifier.setSequenceNumber(sequenceNumber);
+      byte[] password = createPassword(identifier.getBytes(), 
currentKey.getKey());
+      tokenInfo = new DelegationTokenInformation(now + tokenRenewInterval, 
password,
+          getTrackingIdIfEnabled(identifier));
+    } finally {
+      this.apiLock.writeLock().unlock();
+    }
+    LOG.info("Creating password for identifier: " + formatTokenId(identifier)
+        + ", currentKey: " + identifier.getMasterKeyId());

Review Comment:
   AbstractDelegationTokenSecretManager.createPassword(..) already has acquired 
the lock before calling this method.  I wonder if we can print this log message 
outside the lock.



-- 
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]

Reply via email to