hanishakoneru commented on a change in pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#discussion_r435465333
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
##########
@@ -103,6 +103,16 @@
String getOzoneKey(String volume, String bucket, String key);
+ /**
+ * Given a volume, bucket, return the corresponding DB key.
+ *
+ * @param fileHandleInfo - unique keyId that is used by NFS to create a
Review comment:
The Javadoc needs to be updated here. We are not passing the volume and
bucket
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -93,6 +93,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
omMetadataManager.getKeyTable().putWithBatch(batchOperation,
omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
newKeyInfo);
+ // At this point we can also update the KeyIdTable.
+ if (newKeyInfo.getFileHandleInfo() != 0) {
+ omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+ omMetadataManager.getOzoneKeyIdTableKey(
+ newKeyInfo.getFileHandleInfo()), toKeyName);
+ omMetadataManager.getKeyIdTable().deleteWithBatch(batchOperation,
+ omMetadataManager.getOzoneKeyIdTableKey(
+ newKeyInfo.getFileHandleInfo()));
Review comment:
We are putting and deleting the same key from KeyID Table.
The delete operation is not required here as keyId -> fromKey will be
overridden with keyId -> toKey.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -118,12 +118,19 @@
* |----------------------------------------------------------------------|
* | multipartInfoTable| /volumeName/bucketName/keyName/uploadId ->... |
* |----------------------------------------------------------------------|
+ * | keyIdTable | /volumeName/bucketName/keyId -> KeyName |
+ * |----------------------------------------------------------------------|
+ *
+ * TBD : Renames need to be made keyIdTable aware. Also KeyId based lookups
+ * should be able to handle any possible race with renames/deletes.
+ *
*/
public static final String USER_TABLE = "userTable";
public static final String VOLUME_TABLE = "volumeTable";
public static final String BUCKET_TABLE = "bucketTable";
public static final String KEY_TABLE = "keyTable";
+ public static final String KEY_ID_TABLE = "keyIdTable";
Review comment:
Just a thought - should this table be named something else like
FileHandle Table to avoid confusing this with Key Table? (It's just a thought.
Please feel free to ignore if you think "KeyId" is the more appropriate name).
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMDirectoryCreateResponse.java
##########
@@ -81,6 +81,15 @@ protected void addToDBBatch(OMMetadataManager
omMetadataManager,
omMetadataManager.getKeyTable().putWithBatch(batchOperation, dirKey,
dirKeyInfo);
+ // We can also persist the fileHandle to KeyIdTable.
+ if (dirKeyInfo.getFileHandleInfo() != 0) {
+ omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+ omMetadataManager.getOzoneKeyIdTableKey(
+ dirKeyInfo.getFileHandleInfo()),
+ omMetadataManager.getOzoneKey(dirKeyInfo.getVolumeName(),
+ dirKeyInfo.getBucketName(),
+ dirKeyInfo.getKeyName()));
Review comment:
OzoneKey is already processed in line 79 (dirKey). That can be reused
here.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -93,6 +93,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
omMetadataManager.getKeyTable().putWithBatch(batchOperation,
Review comment:
In the if case (deleteFromKeyOnly()) - toKey already exists but fromKey
is created as part of replay. When the fromKey is being created, it would
update KeyID to fromKey value. So KeyId Table should be updated with correct
value (toKey) for this condition also.
@bharatviswa504 is working on replay optimization after which we would not
require to take care of replay scenarios. But let's fix it for now anyway till
the old replay code is still in effect.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java
##########
@@ -156,6 +156,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
.setReplicationFactor(keyArgs.getFactor())
.setObjectID(objectID)
.setUpdateID(transactionLogIndex)
+ .setFileHandleInfo(objectID)
Review comment:
FileHandleInfo in OmMultipartKeyInfo object is not used anywhere.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCreateResponse.java
##########
@@ -87,6 +87,15 @@ protected void addToDBBatch(OMMetadataManager
omMetadataManager,
omKeyInfo.getBucketName(), omKeyInfo.getKeyName(), openKeySessionID);
omMetadataManager.getOpenKeyTable().putWithBatch(batchOperation,
openKey, omKeyInfo);
+
+ // We can also persist the fileHandle to KeyIdTable.
+ if (omKeyInfo.getFileHandleInfo() != 0) {
+ omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+ omMetadataManager.getOzoneKeyIdTableKey(
+ omKeyInfo.getFileHandleInfo()),
+ omMetadataManager.getOzoneKey(omKeyInfo.getVolumeName(),
+ omKeyInfo.getBucketName(), omKeyInfo.getKeyName()));
+ }
Review comment:
This keyId be added when the key is actually getting committed i.e. in
OMKeyCommitResponse#validateAndUpdateCache()? Otherwise, KeyID could point to a
key that does not exist.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3InitiateMultipartUploadResponse.java
##########
@@ -69,6 +69,16 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
multipartKey, omKeyInfo);
omMetadataManager.getMultipartInfoTable().putWithBatch(batchOperation,
multipartKey, omMultipartKeyInfo);
+
+
+ if (omKeyInfo.getFileHandleInfo() != 0) {
+ omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+ omMetadataManager.getOzoneKeyIdTableKey(
+ omKeyInfo.getFileHandleInfo()),
+ omMetadataManager.getOzoneKey(omKeyInfo.getVolumeName(),
+ omKeyInfo.getBucketName(),
+ omKeyInfo.getKeyName()));
+ }
Review comment:
This keyId be added when the key is actually getting committed i.e. in
S3MultipartUploadComplete#validateAndUpdateCache()? Otherwise, KeyID could
point to a key that does not exist.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
