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/06/03 07:04:45 UTC

[GitHub] [hadoop-ozone] bshashikant commented on a change in pull request #958: HDDS-2426. Support recover-trash to an existing bucket.

bshashikant commented on a change in pull request #958:
URL: https://github.com/apache/hadoop-ozone/pull/958#discussion_r434343555



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -932,24 +936,56 @@ private UserVolumeInfo getVolumesByUser(String userNameKey)
         KeyValue<String, RepeatedOmKeyInfo> kv = keyIter.next();
         if (kv != null) {
           RepeatedOmKeyInfo infoList = kv.getValue();
-          // Get block keys as a list.
-          for(OmKeyInfo info : infoList.getOmKeyInfoList()){
-            OmKeyLocationInfoGroup latest = info.getLatestVersionLocations();
-            List<BlockID> item = latest.getLocationList().stream()
-                .map(b -> new BlockID(b.getContainerID(), b.getLocalID()))
-                .collect(Collectors.toList());
-            BlockGroup keyBlocks = BlockGroup.newBuilder()
-                .setKeyName(kv.getKey())
-                .addAllBlockIDs(item)
-                .build();
-            keyBlocksList.add(keyBlocks);
-            currentCount++;
-          }
+          int lastKeyIndex = infoList.getOmKeyInfoList().size() - 1;
+          OmKeyInfo lastKeyInfo = infoList.getOmKeyInfoList().get(lastKeyIndex);
+          /*
+           * Once the last OmKeyInfo is checked to not delete now,
+           * we skip the flow of processing <String, RepeatedOmKeyInfo> here.
+           * This way would keep this <String, RepeatedOmKeyInfo>
+           * from deleting by DB.
+           */
+          if (shouldPendDeleting(lastKeyInfo)) {
+            // Get block keys as a list.
+            for (OmKeyInfo info : infoList.getOmKeyInfoList()) {
+              OmKeyLocationInfoGroup latest = info.getLatestVersionLocations();
+              List<BlockID> item = latest.getLocationList().stream()
+                  .map(b -> new BlockID(b.getContainerID(), b.getLocalID()))
+                  .collect(Collectors.toList());
+              BlockGroup keyBlocks = BlockGroup.newBuilder()
+                  .setKeyName(kv.getKey())
+                  .addAllBlockIDs(item)
+                  .build();
+              keyBlocksList.add(keyBlocks);
+              currentCount++;
+            }
+          } /* else do no-op*/
         }
       }
     }
     return keyBlocksList;
   }
+  /*
+   * Check the key should be deleting by KeyDeletingService in this time or not.
+   *
+   * If the remaining-time of key is more than recover-window,
+   *  it should be deleted.
+   * @return true If key should be deleted.
+   */
+  private boolean shouldPendDeleting(OmKeyInfo keyInfo) throws IOException {

Review comment:
       can we rename the function shouldDelete() ? the name looks confusing 

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMTrashRecoverRequest.java
##########
@@ -93,31 +113,89 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
       acquireLock = omMetadataManager.getLock()
           .acquireWriteLock(BUCKET_LOCK, volumeName, destinationBucket);
 
-      // Validate.
+      // Validate original vol/buc, destinationBucket exists or not.
       validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
       validateBucketAndVolume(omMetadataManager, volumeName, destinationBucket);
 
+      // TODO: HDDS-2425. recovering trash in non-existing bucket.
+
+      String trashTableKey = omMetadataManager
+          .getOzoneKey(volumeName, bucketName, keyName);
+      RepeatedOmKeyInfo trashRepeatedKeyInfo =
+          omMetadataManager.getTrashTable().get(trashTableKey);
+      OmKeyInfo trashKeyInfo = null;
+      if (trashRepeatedKeyInfo != null) {
+        int lastKeyIndex = trashRepeatedKeyInfo.getOmKeyInfoList().size() - 1;
+        trashKeyInfo = trashRepeatedKeyInfo
+            .getOmKeyInfoList().get(lastKeyIndex);
+        // update modificationTime after recovering.
+        trashKeyInfo.setModificationTime(
+            recoverTrashRequest.getModificationTime());
+
+        // Check this transaction is replayed or not.
+        if (isReplay(ozoneManager, trashKeyInfo, transactionLogIndex)) {
+          throw new OMReplayException();
+        }
+
+        // Set the updateID to current transactionLogIndex.
+        trashKeyInfo.setUpdateID(transactionLogIndex,
+            ozoneManager.isRatisEnabled());
+
+        // Update cache of keyTable,
+        if (omMetadataManager.getKeyTable().get(trashTableKey) != null) {
+          throw new OMException(
+              "The bucket has key of same name as recovered key",
+              RECOVERED_KEY_ALREADY_EXISTS);
+        } else {
+          omMetadataManager.getKeyTable().addCacheEntry(
+              new CacheKey<>(trashTableKey),
+              new CacheValue<>(Optional.of(trashKeyInfo), transactionLogIndex));
+        }
+
+        // Update cache of trashTable.
+        trashRepeatedKeyInfo.getOmKeyInfoList().remove(lastKeyIndex);
+        omMetadataManager.getTrashTable().addCacheEntry(
+            new CacheKey<>(trashTableKey),
+            new CacheValue<>(Optional.of(trashRepeatedKeyInfo),
+                transactionLogIndex));
+
+        // Update cache of deletedTable.

Review comment:
       why do we need to update the cache for both trash and deleted table here?
   

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponse.java
##########
@@ -86,6 +86,9 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
             isRatisEnabled);
         omMetadataManager.getDeletedTable().putWithBatch(batchOperation,
             ozoneKey, repeatedOmKeyInfo);
+        // Update trashTable in DB.
+        omMetadataManager.getTrashTable().putWithBatch(batchOperation,

Review comment:
       Ideally, if a key gets deleted from a bucket which is trash enabled, i think we should just add it to the trash table, not the deleted table, otherwise it will lead to having duplicate entries in both trash and delete table. Once the trash interval expires, keys can be moved from trash table to deleted table using a background task which will then be cleaned by the KeyDeleting Service. What do you think?




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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org