smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746092996
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java
##########
@@ -124,32 +140,46 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
boolean acquiredLock = false;
IOException exception = null;
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
- UpdateGetS3SecretRequest updateGetS3SecretRequest =
- getOmRequest().getUpdateGetS3SecretRequest();
- String kerberosID = updateGetS3SecretRequest.getKerberosID();
+
+ final GetS3SecretRequest getS3SecretRequest =
+ getOmRequest().getGetS3SecretRequest();
+ assert(getS3SecretRequest.hasCreateIfNotExist());
+ final boolean createIfNotExist = getS3SecretRequest.getCreateIfNotExist();
+ final String accessId = getS3SecretRequest.getKerberosID();
+ String awsSecret = null;
+ if (createIfNotExist) {
+ final UpdateGetS3SecretRequest updateGetS3SecretRequest =
+ getOmRequest().getUpdateGetS3SecretRequest();
+ awsSecret = updateGetS3SecretRequest.getAwsSecret();
+ assert(accessId.equals(updateGetS3SecretRequest.getKerberosID()));
+ }
+
try {
- String awsSecret = updateGetS3SecretRequest.getAwsSecret();
// Note: We use the same S3_SECRET_LOCK for TenantAccessIdTable.
acquiredLock = omMetadataManager.getLock()
- .acquireWriteLock(S3_SECRET_LOCK, kerberosID);
+ .acquireWriteLock(S3_SECRET_LOCK, accessId);
Review comment:
This change is simply because of the varible renaming, from:
```
String kerberosID = updateGetS3SecretRequest.getKerberosID();
```
to
```
final String accessId = getS3SecretRequest.getKerberosID();
```
This change by itself wouldn't cause any problems. Since it is just a
variable renaming.
We are placing the lock on `accessId` because it is the key of
`S3SecretTable` and `TenantAccessIdTable`. It wouldn't make sense to lock the
tables by **actual** Kerberos ID here. In my understanding, the goal of the
lock is to prevent users from updating the **same** line simultaneously. Not to
prevent the same user from updating the table from multiple clients at the same
time.
--
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]
