You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/01/16 18:48:40 UTC
[GitHub] [hadoop-ozone] hanishakoneru opened a new pull request #452: Hdds
2894
hanishakoneru opened a new pull request #452: Hdds 2894
URL: https://github.com/apache/hadoop-ozone/pull/452
## What changes were proposed in this pull request?
To ensure that key deletion and rename operations are idempotent, compare the transactionID with the objectID and updateID to make sure that the transaction is not a replay. If the transactionID <= updateID, then it implies that the transaction is a replay and hence it should be skipped.
OMKeyDeleteRequest and OMKeyRenameRequest are made idempotent in this Jira.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2894
## How was this patch tested?
Will add Unit test in next commit.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests
URL: https://github.com/apache/hadoop-ozone/pull/452#discussion_r372111521
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
##########
@@ -133,79 +143,146 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
// Validate bucket and volume exists or not.
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
- // fromKeyName should exist
- String fromKey = omMetadataManager.getOzoneKey(
- volumeName, bucketName, fromKeyName);
- fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
- if (fromKeyValue == null) {
- // TODO: Add support for renaming open key
- throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
- }
-
- // toKeyName should not exist
- String toKey =
- omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey = omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
- if (toKeyValue != null) {
- throw new OMException("Key already exists " + toKeyName,
- OMException.ResultCodes.KEY_ALREADY_EXISTS);
- }
-
- fromKeyValue.setKeyName(toKeyName);
- //Set modification time
- fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
-
- // Set the UpdateID to current transactionLogIndex
- fromKeyValue.setUpdateID(transactionLogIndex);
-
- // Add to cache.
- // fromKey should be deleted, toKey should be added with newly updated
- // omKeyInfo.
- Table<String, OmKeyInfo> keyTable = omMetadataManager.getKeyTable();
-
- keyTable.addCacheEntry(new CacheKey<>(fromKey),
- new CacheValue<>(Optional.absent(), transactionLogIndex));
-
- keyTable.addCacheEntry(new CacheKey<>(toKey),
- new CacheValue<>(Optional.of(fromKeyValue), transactionLogIndex));
+ if (toKeyValue != null) {
- omClientResponse = new OMKeyRenameResponse(fromKeyValue, toKeyName,
- fromKeyName, omResponse.setRenameKeyResponse(
- RenameKeyResponse.newBuilder()).build());
- success = true;
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue.getUpdateID(),
+ trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again as Key1 does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and the
+ // request would be deemed a replay. But Key1
+ // is still in the DB and needs to be deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ Table<String, OmKeyInfo> keyTable = omMetadataManager
+ .getKeyTable();
+ keyTable.addCacheEntry(new CacheKey<>(fromKey),
+ new CacheValue<>(Optional.absent(), trxnLogIndex));
+
+ omClientResponse = new OMKeyRenameResponse(omResponse
+ .setRenameKeyResponse(RenameKeyResponse.newBuilder()).build(),
+ fromKeyName, fromKeyValue);
+ }
+ }
+
+ if (result == null) {
Review comment:
Minor: We can write this also else part of fromKeyValue != null. It will be easy to follow the flow.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests
URL: https://github.com/apache/hadoop-ozone/pull/452#discussion_r372111521
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
##########
@@ -133,79 +143,146 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
// Validate bucket and volume exists or not.
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
- // fromKeyName should exist
- String fromKey = omMetadataManager.getOzoneKey(
- volumeName, bucketName, fromKeyName);
- fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
- if (fromKeyValue == null) {
- // TODO: Add support for renaming open key
- throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
- }
-
- // toKeyName should not exist
- String toKey =
- omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey = omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
- if (toKeyValue != null) {
- throw new OMException("Key already exists " + toKeyName,
- OMException.ResultCodes.KEY_ALREADY_EXISTS);
- }
-
- fromKeyValue.setKeyName(toKeyName);
- //Set modification time
- fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
-
- // Set the UpdateID to current transactionLogIndex
- fromKeyValue.setUpdateID(transactionLogIndex);
-
- // Add to cache.
- // fromKey should be deleted, toKey should be added with newly updated
- // omKeyInfo.
- Table<String, OmKeyInfo> keyTable = omMetadataManager.getKeyTable();
-
- keyTable.addCacheEntry(new CacheKey<>(fromKey),
- new CacheValue<>(Optional.absent(), transactionLogIndex));
-
- keyTable.addCacheEntry(new CacheKey<>(toKey),
- new CacheValue<>(Optional.of(fromKeyValue), transactionLogIndex));
+ if (toKeyValue != null) {
- omClientResponse = new OMKeyRenameResponse(fromKeyValue, toKeyName,
- fromKeyName, omResponse.setRenameKeyResponse(
- RenameKeyResponse.newBuilder()).build());
- success = true;
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue.getUpdateID(),
+ trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again as Key1 does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and the
+ // request would be deemed a replay. But Key1
+ // is still in the DB and needs to be deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ Table<String, OmKeyInfo> keyTable = omMetadataManager
+ .getKeyTable();
+ keyTable.addCacheEntry(new CacheKey<>(fromKey),
+ new CacheValue<>(Optional.absent(), trxnLogIndex));
+
+ omClientResponse = new OMKeyRenameResponse(omResponse
+ .setRenameKeyResponse(RenameKeyResponse.newBuilder()).build(),
+ fromKeyName, fromKeyValue);
+ }
+ }
+
+ if (result == null) {
Review comment:
Minor: We can write this also else part of fromKeyValue != null. It will be easy to follow the logic flow.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests
URL: https://github.com/apache/hadoop-ozone/pull/452#discussion_r372111521
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
##########
@@ -133,79 +143,146 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
// Validate bucket and volume exists or not.
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
- // fromKeyName should exist
- String fromKey = omMetadataManager.getOzoneKey(
- volumeName, bucketName, fromKeyName);
- fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
- if (fromKeyValue == null) {
- // TODO: Add support for renaming open key
- throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
- }
-
- // toKeyName should not exist
- String toKey =
- omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey = omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
- if (toKeyValue != null) {
- throw new OMException("Key already exists " + toKeyName,
- OMException.ResultCodes.KEY_ALREADY_EXISTS);
- }
-
- fromKeyValue.setKeyName(toKeyName);
- //Set modification time
- fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
-
- // Set the UpdateID to current transactionLogIndex
- fromKeyValue.setUpdateID(transactionLogIndex);
-
- // Add to cache.
- // fromKey should be deleted, toKey should be added with newly updated
- // omKeyInfo.
- Table<String, OmKeyInfo> keyTable = omMetadataManager.getKeyTable();
-
- keyTable.addCacheEntry(new CacheKey<>(fromKey),
- new CacheValue<>(Optional.absent(), transactionLogIndex));
-
- keyTable.addCacheEntry(new CacheKey<>(toKey),
- new CacheValue<>(Optional.of(fromKeyValue), transactionLogIndex));
+ if (toKeyValue != null) {
- omClientResponse = new OMKeyRenameResponse(fromKeyValue, toKeyName,
- fromKeyName, omResponse.setRenameKeyResponse(
- RenameKeyResponse.newBuilder()).build());
- success = true;
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue.getUpdateID(),
+ trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again as Key1 does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and the
+ // request would be deemed a replay. But Key1
+ // is still in the DB and needs to be deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ Table<String, OmKeyInfo> keyTable = omMetadataManager
+ .getKeyTable();
+ keyTable.addCacheEntry(new CacheKey<>(fromKey),
+ new CacheValue<>(Optional.absent(), trxnLogIndex));
+
+ omClientResponse = new OMKeyRenameResponse(omResponse
+ .setRenameKeyResponse(RenameKeyResponse.newBuilder()).build(),
+ fromKeyName, fromKeyValue);
+ }
+ }
+
+ if (result == null) {
Review comment:
Minor: We can write this also else part of fromKeyValue != null. It will be easy to follow the logic flow.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on issue #452: HDDS-2894.
Handle replay of KeyDelete and KeyRename Requests
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on issue #452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests
URL: https://github.com/apache/hadoop-ozone/pull/452#issuecomment-580020526
Thank You @hanishakoneru for the contribution.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 merged pull request #452: HDDS-2894.
Handle replay of KeyDelete and KeyRename Requests
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests
URL: https://github.com/apache/hadoop-ozone/pull/452
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org