smengcl commented on a change in pull request #2649:
URL: https://github.com/apache/ozone/pull/2649#discussion_r713388143
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java
##########
@@ -126,38 +129,48 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
String kerberosID = updateGetS3SecretRequest.getKerberosID();
try {
String awsSecret = updateGetS3SecretRequest.getAwsSecret();
- acquiredLock =
- omMetadataManager.getLock().acquireWriteLock(S3_SECRET_LOCK,
- kerberosID);
-
- S3SecretValue s3SecretValue =
- omMetadataManager.getS3SecretTable().get(kerberosID);
-
- // If s3Secret for user is not in S3Secret table, add the Secret to
cache.
- if (s3SecretValue == null) {
- omMetadataManager.getS3SecretTable().addCacheEntry(
- new CacheKey<>(kerberosID),
- new CacheValue<>(Optional.of(new S3SecretValue(kerberosID,
- awsSecret)), transactionLogIndex));
+ // Note: We use the same S3_SECRET_LOCK for TenantAccessIdTable.
+ acquiredLock = omMetadataManager.getLock()
+ .acquireWriteLock(S3_SECRET_LOCK, kerberosID);
+
+ // Check multi-tenant table first: tenantAccessIdTable
+ final S3SecretValue assignS3SecretValue;
+ final OmDBAccessIdInfo omDBAccessIdInfo =
+ omMetadataManager.getTenantAccessIdTable().get(kerberosID);
+ if (omDBAccessIdInfo == null) {
+ // Not found in TenantAccessIdTable. Fallback to S3SecretTable.
+ final S3SecretValue s3SecretValue =
+ omMetadataManager.getS3SecretTable().get(kerberosID);
+
+ if (s3SecretValue == null) {
+ // Still not found in S3SecretTable. Will add new entry in this case.
+ assignS3SecretValue = new S3SecretValue(kerberosID, awsSecret);
+ // Add cache entry first.
+ omMetadataManager.getS3SecretTable().addCacheEntry(
+ new CacheKey<>(kerberosID),
+ new CacheValue<>(
+ Optional.of(assignS3SecretValue), transactionLogIndex));
+ } else {
+ // Found in S3SecretTable.
+ awsSecret = s3SecretValue.getAwsSecret();
+ assignS3SecretValue = null;
Review comment:
In this case `assignS3SecretValue` is intentionally `final` so it can
only be assigned once (and enforced this way). I try to use `final` wherever I
can.
--
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]
