From 2ca3f140f655126d882afc458016dca3aadf7dc2 Mon Sep 17 00:00:00 2001 From: captainzmc Date: Tue, 30 Jun 2020 16:37:06 +0800 Subject: [PATCH 01/15] OzoneRpcClient support batch rename keys. --- .../hadoop/ozone/client/OzoneBucket.java | 10 + .../ozone/client/protocol/ClientProtocol.java | 12 +- .../hadoop/ozone/client/rpc/RpcClient.java | 25 +- .../java/org/apache/hadoop/ozone/OmUtils.java | 1 + .../om/protocol/OzoneManagerProtocol.java | 8 + ...ManagerProtocolClientSideTranslatorPB.java | 29 ++ .../rpc/TestOzoneRpcClientAbstract.java | 73 ++++- .../src/main/proto/OmClientProtocol.proto | 11 + .../src/main/resources/proto.lock | 28 ++ .../apache/hadoop/ozone/om/OzoneManager.java | 9 + .../ratis/utils/OzoneManagerRatisUtils.java | 3 + .../om/request/key/OMKeysRenameRequest.java | 308 ++++++++++++++++++ .../om/response/key/OMKeysRenameResponse.java | 108 ++++++ 13 files changed, 617 insertions(+), 8 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java index 79712bbfddb2..d71e03c9b881 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java @@ -472,6 +472,16 @@ public void renameKey(String fromKeyName, String toKeyName) proxy.renameKey(volumeName, name, fromKeyName, toKeyName); } + /** + * Rename the key by keyMap, The key is fromKeyName and value is toKeyName. + * @param keyMap The key is original key name nad value is new key name. + * @throws IOException + */ + public void renameKeys(Map keyMap) + throws IOException { + proxy.renameKeys(volumeName, name, keyMap); + } + /** * Initiate multipart upload for a specified key. * @param keyName diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index 9c662efbf000..1b8d93ac7258 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -314,7 +314,17 @@ void deleteKeys(String volumeName, String bucketName, * @throws IOException */ void renameKey(String volumeName, String bucketName, String fromKeyName, - String toKeyName) throws IOException; + String toKeyName) throws IOException; + + /** + * Renames existing keys within a bucket. + * @param volumeName Name of the Volume + * @param bucketName Name of the Bucket + * @param keyMap The key is original key name nad value is new key name. + * @throws IOException + */ + void renameKeys(String volumeName, String bucketName, + Map keyMap) throws IOException; /** * Returns list of Keys in {Volume/Bucket} that matches the keyPrefix, diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index dc37f095ee69..b4477cc75041 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -25,11 +25,7 @@ import java.net.URI; import java.security.InvalidKeyException; import java.security.SecureRandom; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Map; -import java.util.UUID; +import java.util.*; import java.util.function.Function; import java.util.stream.Collectors; @@ -760,6 +756,25 @@ public void renameKey(String volumeName, String bucketName, ozoneManagerClient.renameKey(keyArgs, toKeyName); } + @Override + public void renameKeys(String volumeName, String bucketName, + Map keyMap) throws IOException { + verifyVolumeName(volumeName); + verifyBucketName(bucketName); + HddsClientUtils.checkNotNull(keyMap); + Map keyArgsMap = new HashMap<>(); + for (Map.Entry< String, String > entry : keyMap.entrySet()) { + OmKeyArgs keyArgs = new OmKeyArgs.Builder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + .setKeyName(entry.getKey()) + .build(); + keyArgsMap.put(entry.getValue(), keyArgs); + } + ozoneManagerClient.renameKeys(keyArgsMap); + } + + @Override public List listKeys(String volumeName, String bucketName, String keyPrefix, String prevKey, diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java index bb9aec4748f2..93e0e7f7dec0 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java @@ -246,6 +246,7 @@ public static boolean isReadOnly( case DeleteBucket: case CreateKey: case RenameKey: + case RenameKeys: case DeleteKey: case DeleteKeys: case CommitKey: diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index 9ae107b071ed..073d0273283b 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -21,6 +21,7 @@ import java.io.Closeable; import java.io.IOException; import java.util.List; +import java.util.Map; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; import org.apache.hadoop.ozone.OzoneAcl; @@ -217,6 +218,13 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID, */ void renameKey(OmKeyArgs args, String toKeyName) throws IOException; + /** + * Rename an existing key within a bucket. + * @param keyMap The key is new key name nad value is original key OmKeyArgs. + * @throws IOException + */ + void renameKeys(Map keyMap) throws IOException; + /** * Deletes an existing key. * diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 1377c53f45b5..57e0772152b7 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -21,6 +21,7 @@ import java.time.Instant; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import org.apache.hadoop.hdds.annotation.InterfaceAudience; @@ -122,6 +123,7 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RemoveAclRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RemoveAclResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenewDelegationTokenResponseProto; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.ServiceListRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.ServiceListResponse; @@ -138,6 +140,7 @@ import org.apache.hadoop.ozone.security.proto.SecurityProtos.GetDelegationTokenRequestProto; import org.apache.hadoop.ozone.security.proto.SecurityProtos.RenewDelegationTokenRequestProto; import org.apache.hadoop.security.token.Token; +import org.apache.hadoop.util.Time; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -675,6 +678,32 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { return OmKeyInfo.getFromProtobuf(resp.getKeyInfo()); } + @Override + public void renameKeys(Map keyMap) throws IOException { + RenameKeysRequest.Builder reqKeys = RenameKeysRequest.newBuilder(); + List renameKeyRequestList = new ArrayList<>(); + for (Map.Entry< String, OmKeyArgs > entry : keyMap.entrySet()) { + RenameKeyRequest.Builder reqKey = RenameKeyRequest.newBuilder(); + OmKeyArgs args = entry.getValue(); + KeyArgs keyArgs = KeyArgs.newBuilder() + .setVolumeName(args.getVolumeName()) + .setBucketName(args.getBucketName()) + .setKeyName(args.getKeyName()) + .setModificationTime(Time.now()) + .setDataSize(args.getDataSize()).build(); + reqKey.setKeyArgs(keyArgs); + reqKey.setToKeyName(entry.getKey()); + renameKeyRequestList.add(reqKey.build()); + } + reqKeys.addAllRenameKeyRequest(renameKeyRequestList); + + OMRequest omRequest = createOMRequest(Type.RenameKeys) + .setRenameKeysRequest(reqKeys) + .build(); + + handleError(submitRequest(omRequest)); + } + @Override public void renameKey(OmKeyArgs args, String toKeyName) throws IOException { RenameKeyRequest.Builder req = RenameKeyRequest.newBuilder(); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java index 32b6bca6a5dd..f8d49584f356 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java @@ -110,6 +110,7 @@ import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS; import static org.apache.hadoop.ozone.OzoneAcl.AclScope.DEFAULT; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.NO_SUCH_MULTIPART_UPLOAD_ERROR; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.GROUP; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.READ; @@ -121,6 +122,8 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; + +import org.junit.Ignore; import org.junit.Test; /** @@ -1202,7 +1205,7 @@ public void testDeleteKey() Assert.assertEquals(keyName, key.getName()); bucket.deleteKey(keyName); - OzoneTestUtils.expectOmException(ResultCodes.KEY_NOT_FOUND, + OzoneTestUtils.expectOmException(KEY_NOT_FOUND, () -> bucket.getKey(keyName)); } @@ -1244,12 +1247,78 @@ public void testRenameKey() } catch (OMException e) { oe = e; } - Assert.assertEquals(ResultCodes.KEY_NOT_FOUND, oe.getResult()); + Assert.assertEquals(KEY_NOT_FOUND, oe.getResult()); key = bucket.getKey(toKeyName); Assert.assertEquals(toKeyName, key.getName()); } + @Test + public void testKeysRename() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + String keyName1 = "dir/file1"; + String keyName2 = "dir/file2"; + + String newKeyName1 = "dir/key1"; + String newKeyName2 = "dir/key2"; + + String value = "sample value"; + store.createVolume(volumeName); + OzoneVolume volume = store.getVolume(volumeName); + volume.createBucket(bucketName); + OzoneBucket bucket = volume.getBucket(bucketName); + OzoneOutputStream out1 = bucket.createKey(keyName1, + value.getBytes().length, STAND_ALONE, + ONE, new HashMap<>()); + out1.write(value.getBytes()); + out1.close(); + OzoneOutputStream out2 = bucket.createKey(keyName2, + value.getBytes().length, STAND_ALONE, + ONE, new HashMap<>()); + out2.write(value.getBytes()); + out2.close(); + OzoneKey key1 = bucket.getKey(keyName1); + OzoneKey key2 = bucket.getKey(keyName2); + Assert.assertEquals(keyName1, key1.getName()); + Assert.assertEquals(keyName2, key2.getName()); + + Map keyMap = new HashMap(); + keyMap.put(keyName1, newKeyName1); + keyMap.put(keyName2, newKeyName2); + bucket.renameKeys(keyMap); + + Assert.assertEquals(newKeyName1, bucket.getKey(newKeyName1).getName()); + Assert.assertEquals(newKeyName2, bucket.getKey(newKeyName2).getName()); + + // old key should not exist + try { + bucket.getKey(keyName1); + } catch (OMException ex) { + Assert.assertEquals(KEY_NOT_FOUND, ex.getResult()); + } + try { + bucket.getKey(keyName2); + } catch (OMException ex) { + Assert.assertEquals(KEY_NOT_FOUND, ex.getResult()); + } + + // rename nonexistent key + Map keyMap1 = new HashMap(); + keyMap1.put(keyName1, keyName2); + keyMap1.put(newKeyName2, keyName2); + try { + bucket.renameKeys(keyMap1); + } catch (OMException ex) { + Assert.assertEquals(KEY_NOT_FOUND, ex.getResult()); + } + + + } + + // Listing all volumes in the cluster feature has to be fixed after HDDS-357. + // TODO: fix this + @Ignore @Test public void testListVolume() throws IOException { String volBase = "vol-" + RandomStringUtils.randomNumeric(3); diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 1b2075e17d9b..5f14613f72e7 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -60,6 +60,7 @@ enum Type { CommitKey = 36; AllocateBlock = 37; DeleteKeys = 38; + RenameKeys = 39; InitiateMultiPartUpload = 45; CommitMultiPartUpload = 46; @@ -126,6 +127,7 @@ message OMRequest { optional CommitKeyRequest commitKeyRequest = 36; optional AllocateBlockRequest allocateBlockRequest = 37; optional DeleteKeysRequest deleteKeysRequest = 38; + optional RenameKeysRequest renameKeysRequest = 39; optional MultipartInfoInitiateRequest initiateMultiPartUploadRequest = 45; optional MultipartCommitUploadPartRequest commitMultiPartUploadRequest = 46; @@ -198,6 +200,7 @@ message OMResponse { optional CommitKeyResponse commitKeyResponse = 36; optional AllocateBlockResponse allocateBlockResponse = 37; optional DeleteKeysResponse deleteKeysResponse = 38; + optional RenameKeysResponse renameKeysResponse = 39; optional MultipartInfoInitiateResponse initiateMultiPartUploadResponse = 45; optional MultipartCommitUploadPartResponse commitMultiPartUploadResponse = 46; @@ -839,6 +842,14 @@ message LookupKeyResponse { optional uint64 openVersion = 4; } +message RenameKeysRequest{ + repeated RenameKeyRequest renameKeyRequest = 1; +} + +message RenameKeysResponse{ + +} + message RenameKeyRequest{ required KeyArgs keyArgs = 1; required string toKeyName = 2; diff --git a/hadoop-ozone/interface-client/src/main/resources/proto.lock b/hadoop-ozone/interface-client/src/main/resources/proto.lock index bef3a0df1e8d..d4951891a5a4 100644 --- a/hadoop-ozone/interface-client/src/main/resources/proto.lock +++ b/hadoop-ozone/interface-client/src/main/resources/proto.lock @@ -83,6 +83,10 @@ "name": "DeleteKeys", "integer": 38 }, + { + "name": "RenameKeys", + "integer": 39 + }, { "name": "InitiateMultiPartUpload", "integer": 45 @@ -711,6 +715,11 @@ "name": "deleteKeysRequest", "type": "DeleteKeysRequest" }, + { + "id": 39, + "name": "renameKeysRequest", + "type": "RenameKeysRequest" + }, { "id": 45, "name": "initiateMultiPartUploadRequest", @@ -982,6 +991,11 @@ "name": "deleteKeysResponse", "type": "DeleteKeysResponse" }, + { + "id": 39, + "name": "renameKeysResponse", + "type": "RenameKeysResponse" + }, { "id": 45, "name": "initiateMultiPartUploadResponse", @@ -2401,6 +2415,20 @@ } ] }, + { + "name": "RenameKeysRequest", + "fields": [ + { + "id": 1, + "name": "renameKeyRequest", + "type": "RenameKeyRequest", + "is_repeated": true + } + ] + }, + { + "name": "RenameKeysResponse" + }, { "name": "RenameKeyRequest", "fields": [ diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 41527a8b4bbc..42ade2ead0fc 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -2222,6 +2222,15 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { } } + + @Override + public void renameKeys(Map omKeyArgsMap) + throws IOException { + for (Map.Entry< String, OmKeyArgs > entry : omKeyArgsMap.entrySet()) { + renameKey(entry.getValue(), entry.getKey()); + } + } + @Override public void renameKey(OmKeyArgs args, String toKeyName) throws IOException { Preconditions.checkNotNull(args); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java index ddb6841ae31e..681c0da87e6d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java @@ -41,6 +41,7 @@ import org.apache.hadoop.ozone.om.request.key.OMKeyDeleteRequest; import org.apache.hadoop.ozone.om.request.key.OMKeyPurgeRequest; import org.apache.hadoop.ozone.om.request.key.OMKeyRenameRequest; +import org.apache.hadoop.ozone.om.request.key.OMKeysRenameRequest; import org.apache.hadoop.ozone.om.request.key.OMTrashRecoverRequest; import org.apache.hadoop.ozone.om.request.key.acl.OMKeyAddAclRequest; import org.apache.hadoop.ozone.om.request.key.acl.OMKeyRemoveAclRequest; @@ -129,6 +130,8 @@ public static OMClientRequest createClientRequest(OMRequest omRequest) { return new OMKeysDeleteRequest(omRequest); case RenameKey: return new OMKeyRenameRequest(omRequest); + case RenameKeys: + return new OMKeysRenameRequest(omRequest); case CreateDirectory: return new OMDirectoryCreateRequest(omRequest); case CreateFile: diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java new file mode 100644 index 000000000000..62ca8ae16acf --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -0,0 +1,308 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om.request.key; + +import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.hdds.utils.db.cache.CacheKey; +import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.audit.AuditLogger; +import org.apache.hadoop.ozone.audit.OMAction; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.OMMetrics; +import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.exceptions.OMException; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; +import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; +import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.om.response.key.OMKeyRenameResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.*; +import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; +import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; + +/** + * Handles rename key request. + */ +public class OMKeysRenameRequest extends OMKeyRequest { + + private static final Logger LOG = + LoggerFactory.getLogger(OMKeysRenameRequest.class); + + public OMKeysRenameRequest(OMRequest omRequest) { + super(omRequest); + } + + /** + * Stores the result of request execution for Rename Requests. + */ + private enum Result { + SUCCESS, + DELETE_FROM_KEY_ONLY, + REPLAY, + FAILURE, + } + + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + + RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest(); + Preconditions.checkNotNull(renameKeysRequest); + + return getOmRequest().toBuilder() + .setRenameKeysRequest(renameKeysRequest.toBuilder()) + .setUserInfo(getUserInfo()).build(); + + } + + @Override + @SuppressWarnings("methodlength") + public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, + long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) { + + RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest(); + OMClientResponse omClientResponse = null; + Set unRenamedKeys = new HashSet<>(); + Set renamedKeys = new HashSet<>(); + + for (RenameKeyRequest renameKeyRequest : renameKeysRequest + .getRenameKeyRequestList()) { + OzoneManagerProtocolProtos.KeyArgs renameKeyArgs = + renameKeyRequest.getKeyArgs(); + unRenamedKeys.add(renameKeyArgs.getKeyName()); + } + + for (RenameKeyRequest renameKeyRequest : renameKeysRequest + .getRenameKeyRequestList()) { + OzoneManagerProtocolProtos.KeyArgs renameKeyArgs = + renameKeyRequest.getKeyArgs(); + + String volumeName = renameKeyArgs.getVolumeName(); + String bucketName = renameKeyArgs.getBucketName(); + String fromKeyName = renameKeyArgs.getKeyName(); + String toKeyName = renameKeyRequest.getToKeyName(); + + OMMetrics omMetrics = ozoneManager.getMetrics(); + omMetrics.incNumKeyRenames(); + + AuditLogger auditLogger = ozoneManager.getAuditLogger(); + + Map auditMap = + buildAuditMap(renameKeyArgs, renameKeyRequest); + + OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder( + getOmRequest()); + + OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager(); + boolean acquiredLock = false; + IOException exception = null; + OmKeyInfo fromKeyValue = null; + String toKey = null, fromKey = null; + Result result = null; + try { + if (toKeyName.length() == 0 || fromKeyName.length() == 0) { + throw new OMException("Key name is empty", + OMException.ResultCodes.INVALID_KEY_NAME); + } + // check Acls to see if user has access to perform delete operation on + // old key and create operation on new key + checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName, + IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY); + checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName, + IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY); + + acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, + volumeName, bucketName); + + // Validate bucket and volume exists or not. + validateBucketAndVolume(omMetadataManager, volumeName, bucketName); + + // 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) { + + // Check if this transaction is a replay of ratis logs. + if (isReplay(ozoneManager, toKeyValue, 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 it 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 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) { + result = Result.REPLAY; + // If toKey exists and fromKey does not, then no further action is + // required. Return a dummy OMClientResponse. + omClientResponse = new OMKeyRenameResponse(createReplayOMResponse( + omResponse)); + } + } else { + // This transaction is not a replay. toKeyName should not exist + throw new OMException("Key already exists " + toKeyName, + OMException.ResultCodes.KEY_ALREADY_EXISTS); + } + } else { + + // This transaction is not a replay. + + // fromKeyName should exist + 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); + } + + fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled()); + + fromKeyValue.setKeyName(toKeyName); + //Set modification time + fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime()); + + // Add to cache. + // fromKey should be deleted, toKey should be added with newly updated + // omKeyInfo. + Table keyTable = omMetadataManager.getKeyTable(); + + keyTable.addCacheEntry(new CacheKey<>(fromKey), + new CacheValue<>(Optional.absent(), trxnLogIndex)); + + keyTable.addCacheEntry(new CacheKey<>(toKey), + new CacheValue<>(Optional.of(fromKeyValue), trxnLogIndex)); + + omClientResponse = new OMKeyRenameResponse(omResponse + .setRenameKeyResponse(RenameKeyResponse.newBuilder()).build(), + fromKeyName, toKeyName, fromKeyValue); + + renamedKeys.add(fromKeyName); + unRenamedKeys.remove(fromKeyName); + result = Result.SUCCESS; + } + } catch (IOException ex) { + result = Result.FAILURE; + exception = ex; + String deleteMessage = String.format( + "The keys that has been renamed form Batch: {%s}.", + StringUtils.join(renamedKeys, ",")); + String unDeleteMessage = String.format( + "The keys that hasn't been renamed form Batch: {%s}.", + StringUtils.join(unRenamedKeys, ",")); + omClientResponse = new OMKeyRenameResponse( + createErrorOMResponse(omResponse, exception)); +// omClientResponse = new OMKeyRenameResponse( +// createOperationKeysErrorOMResponse(omResponse, exception, +// unRenamedKeys)); + } finally { + addResponseToDoubleBuffer(trxnLogIndex, omClientResponse, + omDoubleBufferHelper); + if (acquiredLock) { + omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName, + bucketName); + } + } + + if (result == Result.SUCCESS || result == Result.FAILURE) { + auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap, + exception, getOmRequest().getUserInfo())); + } + + switch (result) { + case SUCCESS: + LOG.debug("Rename Key is successfully completed for volume:{} bucket:{}" + + " fromKey:{} toKey:{}. ", volumeName, bucketName, fromKeyName, + toKeyName); + break; + case DELETE_FROM_KEY_ONLY: + LOG.debug("Replayed transaction {}: {}. Renamed Key {} already exists. " + + "Deleting old key {}.", trxnLogIndex, renameKeyRequest, toKey, + fromKey); + break; + case REPLAY: + LOG.debug("Replayed Transaction {} ignored. Request: {}", trxnLogIndex, + renameKeyRequest); + break; + case FAILURE: + ozoneManager.getMetrics().incNumKeyRenameFails(); + LOG.error("Rename key failed for volume:{} bucket:{} fromKey:{} " + + "toKey:{}. Key: {} not found.", volumeName, bucketName, + fromKeyName, toKeyName, fromKeyName); + break; + default: + LOG.error("Unrecognized Result for OMKeyRenameRequest: {}", + renameKeyRequest); + } + } + + return omClientResponse; + } + + private Map buildAuditMap( + KeyArgs keyArgs, RenameKeyRequest renameKeyRequest) { + Map auditMap = buildKeyArgsAuditMap(keyArgs); + auditMap.remove(OzoneConsts.KEY); + auditMap.put(OzoneConsts.SRC_KEY, keyArgs.getKeyName()); + auditMap.put(OzoneConsts.DST_KEY, renameKeyRequest.getToKeyName()); + return auditMap; + } +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java new file mode 100644 index 000000000000..290bdd156ec4 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java @@ -0,0 +1,108 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om.response.key; + +import com.google.common.annotations.VisibleForTesting; +import org.apache.hadoop.hdds.utils.db.BatchOperation; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; + +import javax.annotation.Nonnull; +import java.io.IOException; + +/** + * Response for RenameKey request. + */ +public class OMKeysRenameResponse extends OMClientResponse { + + private String fromKeyName; + private String toKeyName; + private OmKeyInfo newKeyInfo; + + public OMKeysRenameResponse(@Nonnull OMResponse omResponse, + String fromKeyName, String toKeyName, + @Nonnull OmKeyInfo renameKeyInfo) { + super(omResponse); + this.fromKeyName = fromKeyName; + this.toKeyName = toKeyName; + this.newKeyInfo = renameKeyInfo; + } + + /** + * When Rename request is replayed and toKey already exists, but fromKey + * has not been deleted. + * 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. + */ + public OMKeysRenameResponse(@Nonnull OMResponse omResponse, + String fromKeyName, OmKeyInfo fromKeyInfo) { + super(omResponse); + this.fromKeyName = fromKeyName; + this.newKeyInfo = fromKeyInfo; + this.toKeyName = null; + } + + /** + * For when the request is not successful or it is a replay transaction. + * For a successful request, the other constructor should be used. + */ + public OMKeysRenameResponse(@Nonnull OMResponse omResponse) { + super(omResponse); + checkStatusNotOK(); + } + + @Override + public void addToDBBatch(OMMetadataManager omMetadataManager, + BatchOperation batchOperation) throws IOException { + String volumeName = newKeyInfo.getVolumeName(); + String bucketName = newKeyInfo.getBucketName(); + // If toKeyName is null, then we need to only delete the fromKeyName from + // KeyTable. This is the case of replay where toKey exists but fromKey + // has not been deleted. + if (deleteFromKeyOnly()) { + omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, + omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName)); + } else if (createToKeyAndDeleteFromKey()) { + // If both from and toKeyName are equal do nothing + omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, + omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName)); + omMetadataManager.getKeyTable().putWithBatch(batchOperation, + omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName), + newKeyInfo); + } + } + + @VisibleForTesting + public boolean deleteFromKeyOnly() { + return toKeyName == null && fromKeyName != null; + } + + @VisibleForTesting + public boolean createToKeyAndDeleteFromKey() { + return toKeyName != null && !toKeyName.equals(fromKeyName); + } +} From a282c642e6abefe1ae29558857443fded2e4e157 Mon Sep 17 00:00:00 2001 From: captainzmc Date: Tue, 30 Jun 2020 20:03:24 +0800 Subject: [PATCH 02/15] fix bugs and add unRenamedKeys in response. --- .../hadoop/ozone/client/rpc/RpcClient.java | 7 +- .../om/protocol/OzoneManagerProtocol.java | 2 +- .../rpc/TestOzoneRpcClientAbstract.java | 2 - .../src/main/proto/OmClientProtocol.proto | 2 +- .../src/main/resources/proto.lock | 10 +- .../hadoop/ozone/om/OmRenameKeyInfo.java | 53 ++++ .../ozone/om/request/OMClientRequest.java | 2 + .../om/request/key/OMKeysRenameRequest.java | 251 ++++++++---------- .../om/response/key/OMKeysRenameResponse.java | 115 +++++--- 9 files changed, 259 insertions(+), 185 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index b4477cc75041..349dffd12ebc 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -25,7 +25,12 @@ import java.net.URI; import java.security.InvalidKeyException; import java.security.SecureRandom; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; import java.util.function.Function; import java.util.stream.Collectors; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index 073d0273283b..059d22a6aa51 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -219,7 +219,7 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID, void renameKey(OmKeyArgs args, String toKeyName) throws IOException; /** - * Rename an existing key within a bucket. + * Rename existing keys within a bucket. * @param keyMap The key is new key name nad value is original key OmKeyArgs. * @throws IOException */ diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java index f8d49584f356..ace668b866c2 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java @@ -1312,8 +1312,6 @@ public void testKeysRename() throws Exception { } catch (OMException ex) { Assert.assertEquals(KEY_NOT_FOUND, ex.getResult()); } - - } // Listing all volumes in the cluster feature has to be fixed after HDDS-357. diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 5f14613f72e7..ef2de0ce5527 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -847,7 +847,7 @@ message RenameKeysRequest{ } message RenameKeysResponse{ - + repeated KeyInfo unRenamedKeys = 1; } message RenameKeyRequest{ diff --git a/hadoop-ozone/interface-client/src/main/resources/proto.lock b/hadoop-ozone/interface-client/src/main/resources/proto.lock index d4951891a5a4..81eb08509155 100644 --- a/hadoop-ozone/interface-client/src/main/resources/proto.lock +++ b/hadoop-ozone/interface-client/src/main/resources/proto.lock @@ -2427,7 +2427,15 @@ ] }, { - "name": "RenameKeysResponse" + "name": "RenameKeysResponse", + "fields": [ + { + "id": 1, + "name": "unRenamedKeys", + "type": "KeyInfo", + "is_repeated": true + } + ] }, { "name": "RenameKeyRequest", diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java new file mode 100644 index 000000000000..47d4e9cbd8ce --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java @@ -0,0 +1,53 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om; + +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; + +import javax.annotation.Nonnull; + +/** + * This class is used for rename keys. + */ +public class OmRenameKeyInfo { + + private String toKeyName; + private OmKeyInfo newKeyInfo; + private String fromKeyName; + + public OmRenameKeyInfo(String fromKeyName, String toKeyName, + @Nonnull OmKeyInfo renameKeyInfo) { + this.fromKeyName = fromKeyName; + this.toKeyName = toKeyName; + this.newKeyInfo = renameKeyInfo; + } + + public String getFromKeyName() { + return fromKeyName; + } + + public String getToKeyName() { + return toKeyName; + } + + public OmKeyInfo getNewKeyInfo() { + return newKeyInfo; + } + +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java index 0fa9ca1a8d2c..20140ce103d5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java @@ -36,6 +36,8 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeysResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.security.UserGroupInformation; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 62ca8ae16acf..9cff5c098fb1 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -18,41 +18,44 @@ package org.apache.hadoop.ozone.om.request.key; -import com.google.common.base.Optional; import com.google.common.base.Preconditions; -import org.apache.commons.lang3.StringUtils; -import org.apache.hadoop.hdds.utils.db.Table; -import org.apache.hadoop.hdds.utils.db.cache.CacheKey; -import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; +import org.apache.hadoop.ozone.om.OmRenameKeyInfo; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; import org.apache.hadoop.ozone.om.response.OMClientResponse; -import org.apache.hadoop.ozone.om.response.key.OMKeyRenameResponse; +import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse; +import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.*; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; -import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; /** - * Handles rename key request. + * Handles rename keys request. */ public class OMKeysRenameRequest extends OMKeyRequest { @@ -92,44 +95,57 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest(); OMClientResponse omClientResponse = null; - Set unRenamedKeys = new HashSet<>(); - Set renamedKeys = new HashSet<>(); - - for (RenameKeyRequest renameKeyRequest : renameKeysRequest - .getRenameKeyRequestList()) { - OzoneManagerProtocolProtos.KeyArgs renameKeyArgs = - renameKeyRequest.getKeyArgs(); - unRenamedKeys.add(renameKeyArgs.getKeyName()); - } - - for (RenameKeyRequest renameKeyRequest : renameKeysRequest - .getRenameKeyRequestList()) { - OzoneManagerProtocolProtos.KeyArgs renameKeyArgs = - renameKeyRequest.getKeyArgs(); - - String volumeName = renameKeyArgs.getVolumeName(); - String bucketName = renameKeyArgs.getBucketName(); - String fromKeyName = renameKeyArgs.getKeyName(); - String toKeyName = renameKeyRequest.getToKeyName(); - - OMMetrics omMetrics = ozoneManager.getMetrics(); - omMetrics.incNumKeyRenames(); - - AuditLogger auditLogger = ozoneManager.getAuditLogger(); + Set unRenamedKeys = new HashSet<>(); + List renameKeyInfoList = new ArrayList<>(); + + OMMetrics omMetrics = ozoneManager.getMetrics(); + omMetrics.incNumKeyRenames(); + + AuditLogger auditLogger = ozoneManager.getAuditLogger(); + + + OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder( + getOmRequest()); + + OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager(); + IOException exception = null; + OmKeyInfo fromKeyValue = null; + + Result result = null; + Map auditMap = null; + RenameKeyRequest renameRequest = null; + String toKey = null; + String fromKey = null; + String volumeName = null; + String bucketName = null; + String fromKeyName = null; + String toKeyName = null; + try { + for (RenameKeyRequest renameKeyRequest : renameKeysRequest + .getRenameKeyRequestList()) { + OzoneManagerProtocolProtos.KeyArgs renameKeyArgs = + renameKeyRequest.getKeyArgs(); + volumeName = renameKeyArgs.getVolumeName(); + bucketName = renameKeyArgs.getBucketName(); + fromKeyName = renameKeyArgs.getKeyName(); + String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName, + fromKeyName); + OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey); + unRenamedKeys.add(omKeyInfo); + } - Map auditMap = - buildAuditMap(renameKeyArgs, renameKeyRequest); + for (RenameKeyRequest renameKeyRequest : renameKeysRequest + .getRenameKeyRequestList()) { + OzoneManagerProtocolProtos.KeyArgs renameKeyArgs = + renameKeyRequest.getKeyArgs(); - OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder( - getOmRequest()); + volumeName = renameKeyArgs.getVolumeName(); + bucketName = renameKeyArgs.getBucketName(); + fromKeyName = renameKeyArgs.getKeyName(); + toKeyName = renameKeyRequest.getToKeyName(); + auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest); + renameRequest = renameKeyRequest; - OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager(); - boolean acquiredLock = false; - IOException exception = null; - OmKeyInfo fromKeyValue = null; - String toKey = null, fromKey = null; - Result result = null; - try { if (toKeyName.length() == 0 || fromKeyName.length() == 0) { throw new OMException("Key name is empty", OMException.ResultCodes.INVALID_KEY_NAME); @@ -141,9 +157,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName, IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY); - acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, - volumeName, bucketName); - // Validate bucket and volume exists or not. validateBucketAndVolume(omMetadataManager, volumeName, bucketName); @@ -163,14 +176,14 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, // 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 + // 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 it 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. + // Replay Trxn 1 : Creates Key1 again it 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 @@ -180,14 +193,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, // 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 keyTable = omMetadataManager - .getKeyTable(); - keyTable.addCacheEntry(new CacheKey<>(fromKey), - new CacheValue<>(Optional.absent(), trxnLogIndex)); - - omClientResponse = new OMKeyRenameResponse(omResponse - .setRenameKeyResponse(RenameKeyResponse.newBuilder()) - .build(), fromKeyName, fromKeyValue); + renameKeyInfoList.add(new OmRenameKeyInfo(fromKeyName, + null, fromKeyValue)); } } @@ -195,8 +202,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, result = Result.REPLAY; // If toKey exists and fromKey does not, then no further action is // required. Return a dummy OMClientResponse. - omClientResponse = new OMKeyRenameResponse(createReplayOMResponse( - omResponse)); + omClientResponse = + new OMKeysRenameResponse(createReplayOMResponse( + omResponse)); } } else { // This transaction is not a replay. toKeyName should not exist @@ -204,9 +212,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OMException.ResultCodes.KEY_ALREADY_EXISTS); } } else { - - // This transaction is not a replay. - // fromKeyName should exist fromKeyValue = omMetadataManager.getKeyTable().get(fromKey); if (fromKeyValue == null) { @@ -220,78 +225,54 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, //Set modification time fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime()); - // Add to cache. - // fromKey should be deleted, toKey should be added with newly updated - // omKeyInfo. - Table keyTable = omMetadataManager.getKeyTable(); - - keyTable.addCacheEntry(new CacheKey<>(fromKey), - new CacheValue<>(Optional.absent(), trxnLogIndex)); - - keyTable.addCacheEntry(new CacheKey<>(toKey), - new CacheValue<>(Optional.of(fromKeyValue), trxnLogIndex)); - - omClientResponse = new OMKeyRenameResponse(omResponse - .setRenameKeyResponse(RenameKeyResponse.newBuilder()).build(), - fromKeyName, toKeyName, fromKeyValue); - - renamedKeys.add(fromKeyName); - unRenamedKeys.remove(fromKeyName); - result = Result.SUCCESS; - } - } catch (IOException ex) { - result = Result.FAILURE; - exception = ex; - String deleteMessage = String.format( - "The keys that has been renamed form Batch: {%s}.", - StringUtils.join(renamedKeys, ",")); - String unDeleteMessage = String.format( - "The keys that hasn't been renamed form Batch: {%s}.", - StringUtils.join(unRenamedKeys, ",")); - omClientResponse = new OMKeyRenameResponse( - createErrorOMResponse(omResponse, exception)); -// omClientResponse = new OMKeyRenameResponse( -// createOperationKeysErrorOMResponse(omResponse, exception, -// unRenamedKeys)); - } finally { - addResponseToDoubleBuffer(trxnLogIndex, omClientResponse, - omDoubleBufferHelper); - if (acquiredLock) { - omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName, - bucketName); + renameKeyInfoList + .add(new OmRenameKeyInfo(fromKeyName, toKeyName, fromKeyValue)); } } + omClientResponse = new OMKeysRenameResponse(omResponse + .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(), + renameKeyInfoList, trxnLogIndex); + result = Result.SUCCESS; + } catch (IOException ex) { + result = Result.FAILURE; + exception = ex; + omClientResponse = new OMKeyDeleteResponse( + createRenameKeysErrorOMResponse(omResponse, exception, + unRenamedKeys)); + } finally { + addResponseToDoubleBuffer(trxnLogIndex, omClientResponse, + omDoubleBufferHelper); + } - if (result == Result.SUCCESS || result == Result.FAILURE) { - auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap, - exception, getOmRequest().getUserInfo())); - } + if (result == Result.SUCCESS || result == Result.FAILURE) { + auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap, + exception, getOmRequest().getUserInfo())); + } - switch (result) { - case SUCCESS: - LOG.debug("Rename Key is successfully completed for volume:{} bucket:{}" - + " fromKey:{} toKey:{}. ", volumeName, bucketName, fromKeyName, - toKeyName); - break; - case DELETE_FROM_KEY_ONLY: - LOG.debug("Replayed transaction {}: {}. Renamed Key {} already exists. " - + "Deleting old key {}.", trxnLogIndex, renameKeyRequest, toKey, - fromKey); - break; - case REPLAY: - LOG.debug("Replayed Transaction {} ignored. Request: {}", trxnLogIndex, - renameKeyRequest); - break; - case FAILURE: - ozoneManager.getMetrics().incNumKeyRenameFails(); - LOG.error("Rename key failed for volume:{} bucket:{} fromKey:{} " + - "toKey:{}. Key: {} not found.", volumeName, bucketName, - fromKeyName, toKeyName, fromKeyName); - break; - default: - LOG.error("Unrecognized Result for OMKeyRenameRequest: {}", - renameKeyRequest); - } + switch (result) { + case SUCCESS: + LOG.debug("Rename Key is successfully completed for volume:{} bucket:{}" + + " fromKey:{} toKey:{}. ", volumeName, bucketName, fromKeyName, + toKeyName); + break; + case DELETE_FROM_KEY_ONLY: + LOG.debug("Replayed transaction {}: {}. Renamed Key {} already exists. " + + "Deleting old key {}.", trxnLogIndex, renameRequest, toKey, + fromKey); + break; + case REPLAY: + LOG.debug("Replayed Transaction {} ignored. Request: {}", trxnLogIndex, + renameRequest); + break; + case FAILURE: + ozoneManager.getMetrics().incNumKeyRenameFails(); + LOG.error("Rename key failed for volume:{} bucket:{} fromKey:{} " + + "toKey:{}. Key: {} not found.", volumeName, bucketName, + fromKeyName, toKeyName, fromKeyName); + break; + default: + LOG.error("Unrecognized Result for OMKeyRenameRequest: {}", + renameRequest); } return omClientResponse; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java index 290bdd156ec4..fb7e70037ea3 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java @@ -19,52 +19,44 @@ package org.apache.hadoop.ozone.om.response.key; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Optional; import org.apache.hadoop.hdds.utils.db.BatchOperation; +import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.hdds.utils.db.cache.CacheKey; +import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.OmRenameKeyInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.response.CleanupTableInfo; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import javax.annotation.Nonnull; import java.io.IOException; +import java.util.List; + +import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.KEY_TABLE; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; /** - * Response for RenameKey request. + * Response for RenameKeys request. */ +@CleanupTableInfo(cleanupTables = {KEY_TABLE}) public class OMKeysRenameResponse extends OMClientResponse { - private String fromKeyName; - private String toKeyName; - private OmKeyInfo newKeyInfo; + private List renameKeyInfoList; + private long trxnLogIndex; + private String fromKeyName = null; + private String toKeyName = null; public OMKeysRenameResponse(@Nonnull OMResponse omResponse, - String fromKeyName, String toKeyName, - @Nonnull OmKeyInfo renameKeyInfo) { + List renameKeyInfoList, + long trxnLogIndex) { super(omResponse); - this.fromKeyName = fromKeyName; - this.toKeyName = toKeyName; - this.newKeyInfo = renameKeyInfo; + this.renameKeyInfoList = renameKeyInfoList; + this.trxnLogIndex = trxnLogIndex; } - /** - * When Rename request is replayed and toKey already exists, but fromKey - * has not been deleted. - * 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. - */ - public OMKeysRenameResponse(@Nonnull OMResponse omResponse, - String fromKeyName, OmKeyInfo fromKeyInfo) { - super(omResponse); - this.fromKeyName = fromKeyName; - this.newKeyInfo = fromKeyInfo; - this.toKeyName = null; - } /** * For when the request is not successful or it is a replay transaction. @@ -77,22 +69,57 @@ public OMKeysRenameResponse(@Nonnull OMResponse omResponse) { @Override public void addToDBBatch(OMMetadataManager omMetadataManager, - BatchOperation batchOperation) throws IOException { - String volumeName = newKeyInfo.getVolumeName(); - String bucketName = newKeyInfo.getBucketName(); - // If toKeyName is null, then we need to only delete the fromKeyName from - // KeyTable. This is the case of replay where toKey exists but fromKey - // has not been deleted. - if (deleteFromKeyOnly()) { - omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, - omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName)); - } else if (createToKeyAndDeleteFromKey()) { - // If both from and toKeyName are equal do nothing - omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, - omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName)); - omMetadataManager.getKeyTable().putWithBatch(batchOperation, - omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName), - newKeyInfo); + BatchOperation batchOperation) throws IOException { + boolean acquiredLock = false; + for (OmRenameKeyInfo omRenameKeyInfo : renameKeyInfoList) { + String volumeName = omRenameKeyInfo.getNewKeyInfo().getVolumeName(); + String bucketName = omRenameKeyInfo.getNewKeyInfo().getBucketName(); + fromKeyName = omRenameKeyInfo.getFromKeyName(); + toKeyName = omRenameKeyInfo.getToKeyName(); + OmKeyInfo newKeyInfo = omRenameKeyInfo.getNewKeyInfo(); + Table keyTable = omMetadataManager + .getKeyTable(); + try { + acquiredLock = + omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, + volumeName, bucketName); + // If toKeyName is null, then we need to only delete the fromKeyName + // from KeyTable. This is the case of replay where toKey exists but + // fromKey has not been deleted. + if (deleteFromKeyOnly()) { + + keyTable.addCacheEntry(new CacheKey<>(fromKeyName), + new CacheValue<>(Optional.absent(), trxnLogIndex)); + omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, + omMetadataManager + .getOzoneKey(volumeName, bucketName, fromKeyName)); + } else if (createToKeyAndDeleteFromKey()) { + + // If both from and toKeyName are equal do nothing + keyTable.addCacheEntry(new CacheKey<>(fromKeyName), + new CacheValue<>(Optional.absent(), trxnLogIndex)); + keyTable.addCacheEntry(new CacheKey<>(toKeyName), + new CacheValue<>(Optional.of(newKeyInfo), trxnLogIndex)); + + omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, + omMetadataManager + .getOzoneKey(volumeName, bucketName, fromKeyName)); + omMetadataManager.getKeyTable().putWithBatch(batchOperation, + omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName), + newKeyInfo); + } + if (acquiredLock) { + omMetadataManager.getLock().releaseWriteLock( + BUCKET_LOCK, volumeName, bucketName); + acquiredLock = false; + } + } finally { + if (acquiredLock) { + omMetadataManager.getLock().releaseWriteLock( + BUCKET_LOCK, volumeName, bucketName); + } + } + } } From 483460045f0592888d8801eebca3ca9447633beb Mon Sep 17 00:00:00 2001 From: captainzmc Date: Thu, 2 Jul 2020 18:01:28 +0800 Subject: [PATCH 03/15] fix review issues. --- .../hadoop/ozone/client/rpc/RpcClient.java | 4 ++-- .../om/protocol/OzoneManagerProtocol.java | 4 ++-- ...ManagerProtocolClientSideTranslatorPB.java | 11 ++++----- .../hadoop/ozone/om/OmRenameKeyInfo.java | 8 +------ .../apache/hadoop/ozone/om/OzoneManager.java | 6 ++--- .../om/request/key/OMKeysRenameRequest.java | 23 +++++++++++++------ .../om/response/key/OMKeysRenameResponse.java | 2 +- 7 files changed, 30 insertions(+), 28 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 349dffd12ebc..f54b2b7ee79e 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -767,14 +767,14 @@ public void renameKeys(String volumeName, String bucketName, verifyVolumeName(volumeName); verifyBucketName(bucketName); HddsClientUtils.checkNotNull(keyMap); - Map keyArgsMap = new HashMap<>(); + Map keyArgsMap = new HashMap<>(); for (Map.Entry< String, String > entry : keyMap.entrySet()) { OmKeyArgs keyArgs = new OmKeyArgs.Builder() .setVolumeName(volumeName) .setBucketName(bucketName) .setKeyName(entry.getKey()) .build(); - keyArgsMap.put(entry.getValue(), keyArgs); + keyArgsMap.put(keyArgs, entry.getValue()); } ozoneManagerClient.renameKeys(keyArgsMap); } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index 059d22a6aa51..20957b615300 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -220,10 +220,10 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID, /** * Rename existing keys within a bucket. - * @param keyMap The key is new key name nad value is original key OmKeyArgs. + * @param keyMap The key is original key OmKeyArgs and value is new key name. * @throws IOException */ - void renameKeys(Map keyMap) throws IOException; + void renameKeys(Map keyMap) throws IOException; /** * Deletes an existing key. diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 57e0772152b7..3d93ad5ecb14 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -679,20 +679,19 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { } @Override - public void renameKeys(Map keyMap) throws IOException { + public void renameKeys(Map keyMap) throws IOException { RenameKeysRequest.Builder reqKeys = RenameKeysRequest.newBuilder(); List renameKeyRequestList = new ArrayList<>(); - for (Map.Entry< String, OmKeyArgs > entry : keyMap.entrySet()) { + for (Map.Entry< OmKeyArgs, String> entry : keyMap.entrySet()) { RenameKeyRequest.Builder reqKey = RenameKeyRequest.newBuilder(); - OmKeyArgs args = entry.getValue(); + OmKeyArgs args = entry.getKey(); KeyArgs keyArgs = KeyArgs.newBuilder() .setVolumeName(args.getVolumeName()) .setBucketName(args.getBucketName()) .setKeyName(args.getKeyName()) - .setModificationTime(Time.now()) - .setDataSize(args.getDataSize()).build(); + .setModificationTime(Time.now()).build(); reqKey.setKeyArgs(keyArgs); - reqKey.setToKeyName(entry.getKey()); + reqKey.setToKeyName(entry.getValue()); renameKeyRequestList.add(reqKey.build()); } reqKeys.addAllRenameKeyRequest(renameKeyRequestList); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java index 47d4e9cbd8ce..8b5ed1efced5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java @@ -27,14 +27,12 @@ */ public class OmRenameKeyInfo { - private String toKeyName; private OmKeyInfo newKeyInfo; private String fromKeyName; - public OmRenameKeyInfo(String fromKeyName, String toKeyName, + public OmRenameKeyInfo(String fromKeyName, @Nonnull OmKeyInfo renameKeyInfo) { this.fromKeyName = fromKeyName; - this.toKeyName = toKeyName; this.newKeyInfo = renameKeyInfo; } @@ -42,10 +40,6 @@ public String getFromKeyName() { return fromKeyName; } - public String getToKeyName() { - return toKeyName; - } - public OmKeyInfo getNewKeyInfo() { return newKeyInfo; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 42ade2ead0fc..e89a9d04eee3 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -2224,10 +2224,10 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { @Override - public void renameKeys(Map omKeyArgsMap) + public void renameKeys(Map omKeyArgsMap) throws IOException { - for (Map.Entry< String, OmKeyArgs > entry : omKeyArgsMap.entrySet()) { - renameKey(entry.getValue(), entry.getKey()); + for (Map.Entry entry : omKeyArgsMap.entrySet()) { + renameKey(entry.getKey(), entry.getValue()); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 9cff5c098fb1..d3047b09dd6f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -42,6 +42,7 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.util.Time; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -79,13 +80,21 @@ private enum Result { @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { - RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest(); - Preconditions.checkNotNull(renameKeysRequest); + RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest(); + Preconditions.checkNotNull(renameKeys); - return getOmRequest().toBuilder() - .setRenameKeysRequest(renameKeysRequest.toBuilder()) + List renameKeyList = new ArrayList<>(); + for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) { + // Set modification time. + KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder() + .setModificationTime(Time.now()); + renameKey.toBuilder().setKeyArgs(newKeyArgs); + renameKeyList.add(renameKey); + } + RenameKeysRequest renameKeysRequest = RenameKeysRequest + .newBuilder().addAllRenameKeyRequest(renameKeyList).build(); + return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest) .setUserInfo(getUserInfo()).build(); - } @Override @@ -193,7 +202,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, // 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; - renameKeyInfoList.add(new OmRenameKeyInfo(fromKeyName, + renameKeyInfoList.add(new OmRenameKeyInfo( null, fromKeyValue)); } } @@ -226,7 +235,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime()); renameKeyInfoList - .add(new OmRenameKeyInfo(fromKeyName, toKeyName, fromKeyValue)); + .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue)); } } omClientResponse = new OMKeysRenameResponse(omResponse diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java index fb7e70037ea3..2d507a69dc21 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java @@ -75,8 +75,8 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, String volumeName = omRenameKeyInfo.getNewKeyInfo().getVolumeName(); String bucketName = omRenameKeyInfo.getNewKeyInfo().getBucketName(); fromKeyName = omRenameKeyInfo.getFromKeyName(); - toKeyName = omRenameKeyInfo.getToKeyName(); OmKeyInfo newKeyInfo = omRenameKeyInfo.getNewKeyInfo(); + toKeyName = newKeyInfo.getKeyName(); Table keyTable = omMetadataManager .getKeyTable(); try { From 97fb77b579111c4920165a8267116489d40430e1 Mon Sep 17 00:00:00 2001 From: captainzmc Date: Thu, 2 Jul 2020 22:35:24 +0800 Subject: [PATCH 04/15] fix auditMap Incomplete issue. --- .../om/request/key/OMKeysRenameRequest.java | 30 +++++-------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index d3047b09dd6f..67ab1b540a98 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -19,7 +19,6 @@ package org.apache.hadoop.ozone.om.request.key; import com.google.common.base.Preconditions; -import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; @@ -48,6 +47,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -121,7 +121,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OmKeyInfo fromKeyValue = null; Result result = null; - Map auditMap = null; + Map auditMap = new HashMap<>(); RenameKeyRequest renameRequest = null; String toKey = null; String fromKey = null; @@ -141,6 +141,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, fromKeyName); OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey); unRenamedKeys.add(omKeyInfo); + auditMap.put(fromKeyName, renameKeyRequest.getToKeyName()); } for (RenameKeyRequest renameKeyRequest : renameKeysRequest @@ -152,7 +153,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, bucketName = renameKeyArgs.getBucketName(); fromKeyName = renameKeyArgs.getKeyName(); toKeyName = renameKeyRequest.getToKeyName(); - auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest); renameRequest = renameKeyRequest; if (toKeyName.length() == 0 || fromKeyName.length() == 0) { @@ -260,14 +260,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, switch (result) { case SUCCESS: - LOG.debug("Rename Key is successfully completed for volume:{} bucket:{}" - + " fromKey:{} toKey:{}. ", volumeName, bucketName, fromKeyName, - toKeyName); - break; - case DELETE_FROM_KEY_ONLY: - LOG.debug("Replayed transaction {}: {}. Renamed Key {} already exists. " - + "Deleting old key {}.", trxnLogIndex, renameRequest, toKey, - fromKey); + LOG.debug("Rename Keys is successfully completed for volume:{} bucket:{}" + + " auditMap:{}.", volumeName, bucketName, auditMap.toString()); break; case REPLAY: LOG.debug("Replayed Transaction {} ignored. Request: {}", trxnLogIndex, @@ -275,9 +269,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, break; case FAILURE: ozoneManager.getMetrics().incNumKeyRenameFails(); - LOG.error("Rename key failed for volume:{} bucket:{} fromKey:{} " + - "toKey:{}. Key: {} not found.", volumeName, bucketName, - fromKeyName, toKeyName, fromKeyName); + LOG.error("Rename keys failed for volume:{} bucket:{} auditMap:{}.", + volumeName, bucketName, auditMap.toString()); break; default: LOG.error("Unrecognized Result for OMKeyRenameRequest: {}", @@ -286,13 +279,4 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, return omClientResponse; } - - private Map buildAuditMap( - KeyArgs keyArgs, RenameKeyRequest renameKeyRequest) { - Map auditMap = buildKeyArgsAuditMap(keyArgs); - auditMap.remove(OzoneConsts.KEY); - auditMap.put(OzoneConsts.SRC_KEY, keyArgs.getKeyName()); - auditMap.put(OzoneConsts.DST_KEY, renameKeyRequest.getToKeyName()); - return auditMap; - } } From fa6811fc92ac015e36c31a0311ad4058e28f2c5c Mon Sep 17 00:00:00 2001 From: captainzmc Date: Fri, 3 Jul 2020 19:19:47 +0800 Subject: [PATCH 05/15] add volumeName and bucketName to key. --- .../hadoop/ozone/om/request/key/OMKeysRenameRequest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 67ab1b540a98..1c1868df05b8 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -141,7 +141,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, fromKeyName); OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey); unRenamedKeys.add(omKeyInfo); - auditMap.put(fromKeyName, renameKeyRequest.getToKeyName()); + auditMap.put(volumeName + "/" + bucketName + "/" + fromKeyName, + volumeName + "/" + bucketName + "/" + + renameKeyRequest.getToKeyName()); } for (RenameKeyRequest renameKeyRequest : renameKeysRequest From b243ca04b89c7e82a39c5c92840a46ede1767338 Mon Sep 17 00:00:00 2001 From: captainzmc Date: Mon, 13 Jul 2020 00:11:41 +0800 Subject: [PATCH 06/15] Synchronous HDDS-3930 implementation and fix review issue --- .../org/apache/hadoop/ozone/OzoneConsts.java | 2 + .../apache/hadoop/ozone/audit/OMAction.java | 1 + .../ozone/om/exceptions/OMException.java | 5 +- .../rpc/TestOzoneRpcClientAbstract.java | 3 +- .../src/main/proto/OmClientProtocol.proto | 13 +- .../src/main/resources/proto.lock | 36 ++- .../apache/hadoop/ozone/om/OzoneManager.java | 8 +- .../om/request/key/OMKeysRenameRequest.java | 220 ++++++++++++------ .../om/response/key/OMKeysRenameResponse.java | 66 ++---- .../request/key/TestOMKeysRenameRequest.java | 152 ++++++++++++ .../key/TestOMKeysRenameResponse.java | 131 +++++++++++ 11 files changed, 506 insertions(+), 131 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysRenameRequest.java create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index 4b380948abd9..9854d40494be 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -293,6 +293,8 @@ private OzoneConsts() { public static final String MAX_PARTS = "maxParts"; public static final String S3_BUCKET = "s3Bucket"; public static final String S3_GETSECRET_USER = "S3GetSecretUser"; + public static final String RENAMED_KEYS_MAP = "renamedKeysMap"; + public static final String UNRENAMED_KEYS_MAP = "unRenamedKeysMap"; public static final String MULTIPART_UPLOAD_PART_NUMBER = "partNumber"; public static final String MULTIPART_UPLOAD_PART_NAME = "partName"; public static final String BUCKET_ENCRYPTION_KEY = "bucketEncryptionKey"; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/audit/OMAction.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/audit/OMAction.java index 6b34e8180026..3480063d1323 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/audit/OMAction.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/audit/OMAction.java @@ -32,6 +32,7 @@ public enum OMAction implements AuditAction { DELETE_BUCKET, DELETE_KEY, RENAME_KEY, + RENAME_KEYS, SET_OWNER, SET_QUOTA, UPDATE_VOLUME, diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java index 54b5458af08b..ce0001fcd04e 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java @@ -226,7 +226,10 @@ public enum ResultCodes { PARTIAL_DELETE, DETECTED_LOOP_IN_BUCKET_LINKS, + + NOT_SUPPORTED_OPERATION, + + PARTIAL_RENAME - NOT_SUPPORTED_OPERATION } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java index ace668b866c2..9512cc079fc0 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java @@ -111,6 +111,7 @@ import static org.apache.hadoop.ozone.OzoneAcl.AclScope.DEFAULT; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.NO_SUCH_MULTIPART_UPLOAD_ERROR; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PARTIAL_RENAME; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.GROUP; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.READ; @@ -1310,7 +1311,7 @@ public void testKeysRename() throws Exception { try { bucket.renameKeys(keyMap1); } catch (OMException ex) { - Assert.assertEquals(KEY_NOT_FOUND, ex.getResult()); + Assert.assertEquals(PARTIAL_RENAME, ex.getResult()); } } diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index ef2de0ce5527..e1e062292937 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -311,6 +311,9 @@ enum Status { DETECTED_LOOP_IN_BUCKET_LINKS = 63; NOT_SUPPORTED_OPERATION = 64; + + PARTIAL_RENAME = 65; + } /** @@ -846,8 +849,16 @@ message RenameKeysRequest{ repeated RenameKeyRequest renameKeyRequest = 1; } +message RenameKeyArgs{ + required string volumeName = 1; + required string bucketName = 2; + optional string fromKeyName = 3; + optional string toKeyName = 4; +} + message RenameKeysResponse{ - repeated KeyInfo unRenamedKeys = 1; + repeated RenameKeyArgs unRenamedKeys = 1; + optional bool status = 2; } message RenameKeyRequest{ diff --git a/hadoop-ozone/interface-client/src/main/resources/proto.lock b/hadoop-ozone/interface-client/src/main/resources/proto.lock index 81eb08509155..1a2e6ca1b0f7 100644 --- a/hadoop-ozone/interface-client/src/main/resources/proto.lock +++ b/hadoop-ozone/interface-client/src/main/resources/proto.lock @@ -423,6 +423,10 @@ { "name": "PARTIAL_DELETE", "integer": 62 + }, + { + "name": "PARTIAL_RENAME", + "integer": 64 } ] }, @@ -2426,14 +2430,44 @@ } ] }, + { + "name": "RenameKeyArgs", + "fields": [ + { + "id": 1, + "name": "volumeName", + "type": "string" + }, + { + "id": 2, + "name": "bucketName", + "type": "string" + }, + { + "id": 3, + "name": "fromKeyName", + "type": "string" + }, + { + "id": 4, + "name": "toKeyName", + "type": "string" + } + ] + }, { "name": "RenameKeysResponse", "fields": [ { "id": 1, "name": "unRenamedKeys", - "type": "KeyInfo", + "type": "RenameKeyArgs", "is_repeated": true + }, + { + "id": 2, + "name": "status", + "type": "bool" } ] }, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index e89a9d04eee3..4c89798fd722 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -181,8 +181,9 @@ import com.google.common.base.Preconditions; import com.google.protobuf.BlockingService; import com.google.protobuf.ProtocolMessageEnum; +import org.apache.commons.lang3.NotImplementedException; import org.apache.commons.lang3.StringUtils; - +import org.apache.commons.lang3.tuple.Pair; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_ENABLED; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_ENABLED_DEFAULT; @@ -2226,9 +2227,8 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { @Override public void renameKeys(Map omKeyArgsMap) throws IOException { - for (Map.Entry entry : omKeyArgsMap.entrySet()) { - renameKey(entry.getKey(), entry.getValue()); - } + throw new NotImplementedException("OzoneManager does not require this to " + + "be implemented. As write requests use a new approach"); } @Override diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 1c1868df05b8..7c18e7b9e0f4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -18,24 +18,27 @@ package org.apache.hadoop.ozone.om.request.key; +import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.hdds.utils.db.cache.CacheKey; +import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; import org.apache.hadoop.ozone.om.OmRenameKeyInfo; import org.apache.hadoop.ozone.om.OzoneManager; -import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; import org.apache.hadoop.ozone.om.response.OMClientResponse; -import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse; import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse; @@ -48,12 +51,16 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; -import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME; +import static org.apache.hadoop.ozone.OzoneConsts.BUCKET; +import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP; +import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP; +import static org.apache.hadoop.ozone.OzoneConsts.VOLUME; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; /** * Handles rename keys request. @@ -104,7 +111,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest(); OMClientResponse omClientResponse = null; - Set unRenamedKeys = new HashSet<>(); + // fromKeyName -> toKeyNmae + List unRenamedKeys = new ArrayList<>(); + List renameKeyInfoList = new ArrayList<>(); OMMetrics omMetrics = ozoneManager.getMetrics(); @@ -121,60 +130,68 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OmKeyInfo fromKeyValue = null; Result result = null; - Map auditMap = new HashMap<>(); + Map auditMap = null; RenameKeyRequest renameRequest = null; - String toKey = null; - String fromKey = null; String volumeName = null; String bucketName = null; String fromKeyName = null; String toKeyName = null; + boolean acquiredLock = false; + boolean renameStatus = true; try { for (RenameKeyRequest renameKeyRequest : renameKeysRequest .getRenameKeyRequestList()) { - OzoneManagerProtocolProtos.KeyArgs renameKeyArgs = - renameKeyRequest.getKeyArgs(); - volumeName = renameKeyArgs.getVolumeName(); - bucketName = renameKeyArgs.getBucketName(); - fromKeyName = renameKeyArgs.getKeyName(); - String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName, - fromKeyName); - OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey); - unRenamedKeys.add(omKeyInfo); - auditMap.put(volumeName + "/" + bucketName + "/" + fromKeyName, - volumeName + "/" + bucketName + "/" + - renameKeyRequest.getToKeyName()); - } - - for (RenameKeyRequest renameKeyRequest : renameKeysRequest - .getRenameKeyRequestList()) { - OzoneManagerProtocolProtos.KeyArgs renameKeyArgs = + OzoneManagerProtocolProtos.KeyArgs keyArgs = renameKeyRequest.getKeyArgs(); - volumeName = renameKeyArgs.getVolumeName(); - bucketName = renameKeyArgs.getBucketName(); - fromKeyName = renameKeyArgs.getKeyName(); + volumeName = keyArgs.getVolumeName(); + bucketName = keyArgs.getBucketName(); + fromKeyName = keyArgs.getKeyName(); toKeyName = renameKeyRequest.getToKeyName(); renameRequest = renameKeyRequest; + RenameKeyArgs renameKeyArgs = RenameKeyArgs.newBuilder() + .setVolumeName(volumeName).setBucketName(bucketName) + .setFromKeyName(fromKeyName).setToKeyName(toKeyName).build(); + + try { + // Validate bucket and volume exists or not. + validateBucketAndVolume(omMetadataManager, volumeName, bucketName); + } catch (Exception ex) { + renameStatus = false; + unRenamedKeys.add(renameKeyArgs); + LOG.error("Validate bucket and volume exists failed" + + "volumeName {} bucketName {}", volumeName, bucketName, ex); + continue; + } + if (toKeyName.length() == 0 || fromKeyName.length() == 0) { - throw new OMException("Key name is empty", - OMException.ResultCodes.INVALID_KEY_NAME); + renameStatus = false; + unRenamedKeys.add(renameKeyArgs); + LOG.error("Key name is empty fromKeyName {} toKeyName {}", + fromKeyName, toKeyName); + continue; } - // check Acls to see if user has access to perform delete operation on - // old key and create operation on new key - checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName, - IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY); - checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName, - IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY); - // Validate bucket and volume exists or not. - validateBucketAndVolume(omMetadataManager, volumeName, bucketName); + try { + // check Acls to see if user has access to perform delete operation + // on old key and create operation on new key + checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName, + IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY); + checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName, + IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY); + } catch (Exception ex) { + renameStatus = false; + unRenamedKeys.add(renameKeyArgs); + LOG.error("Acl check failed for fromKeyName {} toKeyName {}", + fromKeyName, toKeyName, ex); + continue; + } // Check if toKey exists - fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName, + String fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName); - toKey = + String toKey = omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName); OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey); @@ -201,69 +218,99 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, // created. If so, we have to delete the fromKey. if (ozoneManager.isRatisEnabled() && trxnLogIndex > fromKeyValue.getUpdateID()) { + acquiredLock = + omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, + volumeName, bucketName); // 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 keyTable = omMetadataManager + .getKeyTable(); + keyTable.addCacheEntry(new CacheKey<>(fromKey), + new CacheValue<>(Optional.absent(), trxnLogIndex)); renameKeyInfoList.add(new OmRenameKeyInfo( null, fromKeyValue)); } } - - if (result == null) { - result = Result.REPLAY; - // If toKey exists and fromKey does not, then no further action is - // required. Return a dummy OMClientResponse. - omClientResponse = - new OMKeysRenameResponse(createReplayOMResponse( - omResponse)); - } } else { - // This transaction is not a replay. toKeyName should not exist - throw new OMException("Key already exists " + toKeyName, - OMException.ResultCodes.KEY_ALREADY_EXISTS); + renameStatus = false; + unRenamedKeys.add(renameKeyArgs); + LOG.error("Received a request name of new key {} already exists", + toKeyName); } } else { // fromKeyName should exist 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); + renameStatus = false; + unRenamedKeys.add(renameKeyArgs); + LOG.error("Received a request to rename a Key does not exist {}", + fromKey); + continue; } fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled()); - fromKeyValue.setKeyName(toKeyName); //Set modification time - fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime()); - + fromKeyValue.setModificationTime(keyArgs.getModificationTime()); + + acquiredLock = + omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, + volumeName, bucketName); + // Add to cache. + // fromKey should be deleted, toKey should be added with newly updated + // omKeyInfo. + Table keyTable = omMetadataManager.getKeyTable(); + keyTable.addCacheEntry(new CacheKey<>(fromKey), + new CacheValue<>(Optional.absent(), trxnLogIndex)); + keyTable.addCacheEntry(new CacheKey<>(toKey), + new CacheValue<>(Optional.of(fromKeyValue), trxnLogIndex)); renameKeyInfoList .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue)); } + if (acquiredLock) { + omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName, + bucketName); + acquiredLock = false; + } } + acquiredLock = + omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, + volumeName, bucketName); omClientResponse = new OMKeysRenameResponse(omResponse - .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(), - renameKeyInfoList, trxnLogIndex); + .setRenameKeysResponse(RenameKeysResponse.newBuilder() + .setStatus(renameStatus).addAllUnRenamedKeys(unRenamedKeys)) + .setStatus(renameStatus ? OK : PARTIAL_RENAME) + .setSuccess(renameStatus).build(), + renameKeyInfoList); + result = Result.SUCCESS; } catch (IOException ex) { result = Result.FAILURE; exception = ex; - omClientResponse = new OMKeyDeleteResponse( - createRenameKeysErrorOMResponse(omResponse, exception, - unRenamedKeys)); + createErrorOMResponse(omResponse, ex); + + omResponse.setRenameKeysResponse(RenameKeysResponse.newBuilder() + .setStatus(renameStatus).addAllUnRenamedKeys(unRenamedKeys).build()); + omClientResponse = new OMKeysRenameResponse(omResponse.build()); + } finally { + if (acquiredLock) { + omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName, + bucketName); + } addResponseToDoubleBuffer(trxnLogIndex, omClientResponse, omDoubleBufferHelper); } - if (result == Result.SUCCESS || result == Result.FAILURE) { - auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap, - exception, getOmRequest().getUserInfo())); - } + auditMap = buildAuditMap(volumeName, bucketName, renameKeyInfoList, + unRenamedKeys); + auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEYS, auditMap, + exception, getOmRequest().getUserInfo())); switch (result) { case SUCCESS: - LOG.debug("Rename Keys is successfully completed for volume:{} bucket:{}" - + " auditMap:{}.", volumeName, bucketName, auditMap.toString()); + LOG.debug("Rename Keys is successfully completed for auditMap:{}.", + auditMap.toString()); break; case REPLAY: LOG.debug("Replayed Transaction {} ignored. Request: {}", trxnLogIndex, @@ -271,8 +318,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, break; case FAILURE: ozoneManager.getMetrics().incNumKeyRenameFails(); - LOG.error("Rename keys failed for volume:{} bucket:{} auditMap:{}.", - volumeName, bucketName, auditMap.toString()); + LOG.error("Rename keys failed for auditMap:{}.", auditMap.toString()); break; default: LOG.error("Unrecognized Result for OMKeyRenameRequest: {}", @@ -281,4 +327,34 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, return omClientResponse; } + + /** + * Build audit map for RenameKeys request. + * @param volumeName + * @param bucketName + * @param renameKeys + * @param unRenameKeys + * @return + */ + private Map buildAuditMap(String volumeName, + String bucketName, List renameKeys, + List unRenameKeys) { + Map renameKeysMap = new HashMap<>(); + Map unRenameKeysMap = new HashMap<>(); + Map auditMap = new HashMap<>(); + + for(OmRenameKeyInfo keyInfo : renameKeys) { + renameKeysMap.put(keyInfo.getFromKeyName(), + keyInfo.getNewKeyInfo().getKeyName()); + } + for(RenameKeyArgs keyArgs : unRenameKeys) { + unRenameKeysMap.put(keyArgs.getFromKeyName(), keyArgs.getToKeyName()); + } + + auditMap.put(VOLUME, volumeName); + auditMap.put(BUCKET, bucketName); + auditMap.put(RENAMED_KEYS_MAP, renameKeysMap.toString()); + auditMap.put(UNRENAMED_KEYS_MAP, unRenameKeysMap.toString()); + return auditMap; + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java index 2d507a69dc21..8e23fad008d4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java @@ -19,11 +19,7 @@ package org.apache.hadoop.ozone.om.response.key; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Optional; import org.apache.hadoop.hdds.utils.db.BatchOperation; -import org.apache.hadoop.hdds.utils.db.Table; -import org.apache.hadoop.hdds.utils.db.cache.CacheKey; -import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OmRenameKeyInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; @@ -36,7 +32,6 @@ import java.util.List; import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.KEY_TABLE; -import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; /** * Response for RenameKeys request. @@ -45,16 +40,13 @@ public class OMKeysRenameResponse extends OMClientResponse { private List renameKeyInfoList; - private long trxnLogIndex; private String fromKeyName = null; private String toKeyName = null; public OMKeysRenameResponse(@Nonnull OMResponse omResponse, - List renameKeyInfoList, - long trxnLogIndex) { + List renameKeyInfoList) { super(omResponse); this.renameKeyInfoList = renameKeyInfoList; - this.trxnLogIndex = trxnLogIndex; } @@ -70,54 +62,26 @@ public OMKeysRenameResponse(@Nonnull OMResponse omResponse) { @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { - boolean acquiredLock = false; for (OmRenameKeyInfo omRenameKeyInfo : renameKeyInfoList) { String volumeName = omRenameKeyInfo.getNewKeyInfo().getVolumeName(); String bucketName = omRenameKeyInfo.getNewKeyInfo().getBucketName(); fromKeyName = omRenameKeyInfo.getFromKeyName(); OmKeyInfo newKeyInfo = omRenameKeyInfo.getNewKeyInfo(); toKeyName = newKeyInfo.getKeyName(); - Table keyTable = omMetadataManager - .getKeyTable(); - try { - acquiredLock = - omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, - volumeName, bucketName); - // If toKeyName is null, then we need to only delete the fromKeyName - // from KeyTable. This is the case of replay where toKey exists but - // fromKey has not been deleted. - if (deleteFromKeyOnly()) { - - keyTable.addCacheEntry(new CacheKey<>(fromKeyName), - new CacheValue<>(Optional.absent(), trxnLogIndex)); - omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, - omMetadataManager - .getOzoneKey(volumeName, bucketName, fromKeyName)); - } else if (createToKeyAndDeleteFromKey()) { - - // If both from and toKeyName are equal do nothing - keyTable.addCacheEntry(new CacheKey<>(fromKeyName), - new CacheValue<>(Optional.absent(), trxnLogIndex)); - keyTable.addCacheEntry(new CacheKey<>(toKeyName), - new CacheValue<>(Optional.of(newKeyInfo), trxnLogIndex)); - - omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, - omMetadataManager - .getOzoneKey(volumeName, bucketName, fromKeyName)); - omMetadataManager.getKeyTable().putWithBatch(batchOperation, - omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName), - newKeyInfo); - } - if (acquiredLock) { - omMetadataManager.getLock().releaseWriteLock( - BUCKET_LOCK, volumeName, bucketName); - acquiredLock = false; - } - } finally { - if (acquiredLock) { - omMetadataManager.getLock().releaseWriteLock( - BUCKET_LOCK, volumeName, bucketName); - } + // If toKeyName is null, then we need to only delete the fromKeyName + // from KeyTable. This is the case of replay where toKey exists but + // fromKey has not been deleted. + if (deleteFromKeyOnly()) { + omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, + omMetadataManager + .getOzoneKey(volumeName, bucketName, fromKeyName)); + } else if (createToKeyAndDeleteFromKey()) { + omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, + omMetadataManager + .getOzoneKey(volumeName, bucketName, fromKeyName)); + omMetadataManager.getKeyTable().putWithBatch(batchOperation, + omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName), + newKeyInfo); } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysRenameRequest.java new file mode 100644 index 000000000000..93657a71268d --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysRenameRequest.java @@ -0,0 +1,152 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om.request.key; + +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; +import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest; +import org.junit.Assert; +import org.junit.Test; + +import java.util.UUID; + +/** + * Tests RenameKey request. + */ +public class TestOMKeysRenameRequest extends TestOMKeyRequest { + + private int count = 10; + private String parentDir = "/test"; + + @Test + public void testKeysRenameRequest() throws Exception { + + OMRequest modifiedOmRequest = createRenameKeyRequest(false); + + OMKeysRenameRequest omKeysRenameRequest = + new OMKeysRenameRequest(modifiedOmRequest); + + OMClientResponse omKeysRenameResponse = + omKeysRenameRequest.validateAndUpdateCache(ozoneManager, 100L, + ozoneManagerDoubleBufferHelper); + + Assert.assertTrue(omKeysRenameResponse.getOMResponse().getSuccess()); + Assert.assertEquals(OzoneManagerProtocolProtos.Status.OK, + omKeysRenameResponse.getOMResponse().getStatus()); + + for (int i = 0; i < count; i++) { + // Original key should be deleted, toKey should exist. + OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get( + omMetadataManager.getOzoneKey(volumeName, bucketName, + parentDir.concat("/key" + i))); + Assert.assertNull(omKeyInfo); + + omKeyInfo = + omMetadataManager.getKeyTable().get(omMetadataManager.getOzoneKey( + volumeName, bucketName, parentDir.concat("/newKey" + i))); + Assert.assertNotNull(omKeyInfo); + } + + } + + @Test + public void testKeysRenameRequestFail() throws Exception { + OMRequest modifiedOmRequest = createRenameKeyRequest(true); + + OMKeysRenameRequest omKeysRenameRequest = + new OMKeysRenameRequest(modifiedOmRequest); + + OMClientResponse omKeysRenameResponse = + omKeysRenameRequest.validateAndUpdateCache(ozoneManager, 100L, + ozoneManagerDoubleBufferHelper); + + Assert.assertFalse(omKeysRenameResponse.getOMResponse().getSuccess()); + Assert.assertEquals(OzoneManagerProtocolProtos.Status.PARTIAL_RENAME, + omKeysRenameResponse.getOMResponse().getStatus()); + + // The keys（key0 to key9）can be renamed success. + for (int i = 0; i < count; i++) { + // Original key should be deleted, toKey should exist. + OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get( + omMetadataManager.getOzoneKey(volumeName, bucketName, + parentDir.concat("/key" + i))); + Assert.assertNull(omKeyInfo); + + omKeyInfo = + omMetadataManager.getKeyTable().get(omMetadataManager.getOzoneKey( + volumeName, bucketName, parentDir.concat("/newKey" + i))); + Assert.assertNotNull(omKeyInfo); + } + + // The key not rename should be in unRenamedKeys. + RenameKeyArgs unRenamedKeys = omKeysRenameResponse.getOMResponse() + .getRenameKeysResponse().getUnRenamedKeys(0); + Assert.assertEquals("testKey", unRenamedKeys.getFromKeyName()); + } + + /** + * Create OMRequest which encapsulates RenameKeyRequest. + * + * @return OMRequest + */ + private OMRequest createRenameKeyRequest(Boolean isIllegal) throws Exception { + + RenameKeysRequest.Builder renameKeysReq = RenameKeysRequest.newBuilder(); + + // Add volume, bucket and key entries to OM DB. + TestOMRequestUtils.addVolumeAndBucketToDB(volumeName, bucketName, + omMetadataManager); + + for (int i = 0; i < count; i++) { + String key = parentDir.concat("/key" + i); + String toKey = parentDir.concat("/newKey" + i); + TestOMRequestUtils.addKeyToTableCache(volumeName, bucketName, + parentDir.concat("/key" + i), HddsProtos.ReplicationType.RATIS, + HddsProtos.ReplicationFactor.THREE, omMetadataManager); + + KeyArgs keyArgs = KeyArgs.newBuilder().setKeyName(key) + .setVolumeName(volumeName).setBucketName(bucketName).build(); + RenameKeyRequest renameKeyRequest = RenameKeyRequest.newBuilder() + .setKeyArgs(keyArgs).setToKeyName(toKey).build(); + renameKeysReq.addRenameKeyRequest(renameKeyRequest); + } + + // Generating illegal data causes Rename Keys to fail. + if (isIllegal) { + KeyArgs keyArgs = KeyArgs.newBuilder().setKeyName("testKey") + .setVolumeName(volumeName).setBucketName(bucketName).build(); + RenameKeyRequest renameKeyRequest = RenameKeyRequest.newBuilder() + .setKeyArgs(keyArgs).setToKeyName("testToKey").build(); + renameKeysReq.addRenameKeyRequest(renameKeyRequest); + } + + return OMRequest.newBuilder() + .setClientId(UUID.randomUUID().toString()) + .setRenameKeysRequest(renameKeysReq.build()) + .setCmdType(OzoneManagerProtocolProtos.Type.RenameKeys).build(); + } + +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java new file mode 100644 index 000000000000..18223e61b9ae --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java @@ -0,0 +1,131 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om.response.key; + +import org.apache.hadoop.ozone.om.OmRenameKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType.RATIS; + +/** + * Tests OMKeyRenameResponse. + */ +public class TestOMKeysRenameResponse extends TestOMKeyResponse { + private List renameKeyInfoList; + private int count = 10; + private String parentDir = "/test"; + + @Test + public void testKeysRenameResponse() throws Exception { + + createPreRequisities(); + + OMResponse omResponse = OMResponse.newBuilder() + .setRenameKeysResponse(RenameKeysResponse.getDefaultInstance()) + .setStatus(Status.OK).setCmdType(Type.RenameKeys).build(); + + OMKeysRenameResponse omKeysRenameResponse = new OMKeysRenameResponse( + omResponse, renameKeyInfoList); + + omKeysRenameResponse.addToDBBatch(omMetadataManager, batchOperation); + + // Do manual commit and see whether addToBatch is successful or not. + omMetadataManager.getStore().commitBatchOperation(batchOperation); + + // Add volume, bucket and key entries to OM DB. + TestOMRequestUtils.addVolumeAndBucketToDB(volumeName, bucketName, + omMetadataManager); + + for (int i = 0; i < count; i++) { + String key = parentDir.concat("/key" + i); + String toKey = parentDir.concat("/newKey" + i); + key = omMetadataManager.getOzoneKey(volumeName, bucketName, key); + toKey = omMetadataManager.getOzoneKey(volumeName, bucketName, toKey); + Assert.assertFalse(omMetadataManager.getKeyTable().isExist(key)); + Assert.assertTrue(omMetadataManager.getKeyTable().isExist(toKey)); + } + } + + @Test + public void testKeysRenameResponseFail() throws Exception { + + createPreRequisities(); + + OMResponse omResponse = OMResponse.newBuilder().setRenameKeysResponse( + RenameKeysResponse.getDefaultInstance()) + .setStatus(Status.KEY_NOT_FOUND) + .setCmdType(Type.RenameKeys) + .build(); + + OMKeysRenameResponse omKeyRenameResponse = new OMKeysRenameResponse( + omResponse, renameKeyInfoList); + + omKeyRenameResponse.checkAndUpdateDB(omMetadataManager, batchOperation); + + // Do manual commit and see whether addToBatch is successful or not. + omMetadataManager.getStore().commitBatchOperation(batchOperation); + + for (int i = 0; i < count; i++) { + String key = parentDir.concat("/key" + i); + String toKey = parentDir.concat("/newKey" + i); + key = omMetadataManager.getOzoneKey(volumeName, bucketName, key); + toKey = omMetadataManager.getOzoneKey(volumeName, bucketName, toKey); + // As omResponse has error, it is a no-op. So, no changes should happen. + Assert.assertTrue(omMetadataManager.getKeyTable().isExist(key)); + Assert.assertFalse(omMetadataManager.getKeyTable().isExist(toKey)); + } + + + } + + private void createPreRequisities() throws Exception { + + renameKeyInfoList = new ArrayList<>(); + // Add volume, bucket and key entries to OM DB. + TestOMRequestUtils.addVolumeAndBucketToDB(volumeName, bucketName, + omMetadataManager); + + for (int i = 0; i < count; i++) { + String key = parentDir.concat("/key" + i); + String toKey = parentDir.concat("/newKey" + i); + TestOMRequestUtils.addKeyToTable(false, volumeName, + bucketName, parentDir.concat("/key" + i), 0L, RATIS, THREE, + omMetadataManager); + + OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get( + omMetadataManager.getOzoneKey(volumeName, bucketName, key)); + omKeyInfo.setKeyName(toKey); + OmRenameKeyInfo omRenameKeyInfo = new OmRenameKeyInfo(key, omKeyInfo); + renameKeyInfoList.add(omRenameKeyInfo); + } + + } +} From c5bc4915310d0928e56fa14c2aa2594786941129 Mon Sep 17 00:00:00 2001 From: captainzmc Date: Wed, 15 Jul 2020 17:52:42 +0800 Subject: [PATCH 07/15] Synchronous HDDS-3685 Remove replay logic from keysRename --- .../src/main/resources/proto.lock | 2 +- .../ozone/om/request/OMClientRequest.java | 2 - .../om/request/key/OMKeysRenameRequest.java | 135 ++++++------------ .../om/response/key/OMKeysRenameResponse.java | 32 +---- 4 files changed, 53 insertions(+), 118 deletions(-) diff --git a/hadoop-hdds/interface-client/src/main/resources/proto.lock b/hadoop-hdds/interface-client/src/main/resources/proto.lock index b27896c655e3..5ba0625f33fb 100644 --- a/hadoop-hdds/interface-client/src/main/resources/proto.lock +++ b/hadoop-hdds/interface-client/src/main/resources/proto.lock @@ -1935,4 +1935,4 @@ } } ] -} +} \ No newline at end of file diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java index 20140ce103d5..0fa9ca1a8d2c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java @@ -36,8 +36,6 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeysResponse; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.security.UserGroupInformation; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 7c18e7b9e0f4..339b1641804c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -74,16 +74,6 @@ public OMKeysRenameRequest(OMRequest omRequest) { super(omRequest); } - /** - * Stores the result of request execution for Rename Requests. - */ - private enum Result { - SUCCESS, - DELETE_FROM_KEY_ONLY, - REPLAY, - FAILURE, - } - @Override public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { @@ -197,76 +187,43 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (toKeyValue != null) { - // Check if this transaction is a replay of ratis logs. - if (isReplay(ozoneManager, toKeyValue, 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 it 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()) { - acquiredLock = - omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, - volumeName, bucketName); - // Add to cache. Only fromKey should be deleted. ToKey already - // exists in DB as this transaction is a replay. - Table keyTable = omMetadataManager - .getKeyTable(); - keyTable.addCacheEntry(new CacheKey<>(fromKey), - new CacheValue<>(Optional.absent(), trxnLogIndex)); - renameKeyInfoList.add(new OmRenameKeyInfo( - null, fromKeyValue)); - } - } - } else { - renameStatus = false; - unRenamedKeys.add(renameKeyArgs); - LOG.error("Received a request name of new key {} already exists", - toKeyName); - } - } else { - // fromKeyName should exist - fromKeyValue = omMetadataManager.getKeyTable().get(fromKey); - if (fromKeyValue == null) { - renameStatus = false; - unRenamedKeys.add(renameKeyArgs); - LOG.error("Received a request to rename a Key does not exist {}", - fromKey); - continue; - } - - fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled()); - fromKeyValue.setKeyName(toKeyName); - //Set modification time - fromKeyValue.setModificationTime(keyArgs.getModificationTime()); - - acquiredLock = - omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, - volumeName, bucketName); - // Add to cache. - // fromKey should be deleted, toKey should be added with newly updated - // omKeyInfo. - Table keyTable = omMetadataManager.getKeyTable(); - keyTable.addCacheEntry(new CacheKey<>(fromKey), - new CacheValue<>(Optional.absent(), trxnLogIndex)); - keyTable.addCacheEntry(new CacheKey<>(toKey), - new CacheValue<>(Optional.of(fromKeyValue), trxnLogIndex)); - renameKeyInfoList - .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue)); + renameStatus = false; + unRenamedKeys.add(renameKeyArgs); + LOG.error("Received a request name of new key {} already exists", + toKeyName); + } + + // fromKeyName should exist + fromKeyValue = omMetadataManager.getKeyTable().get(fromKey); + if (fromKeyValue == null) { + renameStatus = false; + unRenamedKeys.add(renameKeyArgs); + LOG.error("Received a request to rename a Key does not exist {}", + fromKey); + continue; } + + fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled()); + + fromKeyValue.setKeyName(toKeyName); + + //Set modification time + fromKeyValue.setModificationTime(keyArgs.getModificationTime()); + + acquiredLock = + omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, + volumeName, bucketName); + // Add to cache. + // fromKey should be deleted, toKey should be added with newly updated + // omKeyInfo. + Table keyTable = omMetadataManager.getKeyTable(); + keyTable.addCacheEntry(new CacheKey<>(fromKey), + new CacheValue<>(Optional.absent(), trxnLogIndex)); + keyTable.addCacheEntry(new CacheKey<>(toKey), + new CacheValue<>(Optional.of(fromKeyValue), trxnLogIndex)); + renameKeyInfoList + .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue)); + if (acquiredLock) { omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName, bucketName); @@ -278,7 +235,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, volumeName, bucketName); omClientResponse = new OMKeysRenameResponse(omResponse .setRenameKeysResponse(RenameKeysResponse.newBuilder() - .setStatus(renameStatus).addAllUnRenamedKeys(unRenamedKeys)) + .setStatus(renameStatus).addAllUnRenamedKeys(unRenamedKeys)) .setStatus(renameStatus ? OK : PARTIAL_RENAME) .setSuccess(renameStatus).build(), renameKeyInfoList); @@ -294,12 +251,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, omClientResponse = new OMKeysRenameResponse(omResponse.build()); } finally { + addResponseToDoubleBuffer(trxnLogIndex, omClientResponse, + omDoubleBufferHelper); if (acquiredLock) { omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName, bucketName); } - addResponseToDoubleBuffer(trxnLogIndex, omClientResponse, - omDoubleBufferHelper); } auditMap = buildAuditMap(volumeName, bucketName, renameKeyInfoList, @@ -312,10 +269,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, LOG.debug("Rename Keys is successfully completed for auditMap:{}.", auditMap.toString()); break; - case REPLAY: - LOG.debug("Replayed Transaction {} ignored. Request: {}", trxnLogIndex, - renameRequest); - break; case FAILURE: ozoneManager.getMetrics().incNumKeyRenameFails(); LOG.error("Rename keys failed for auditMap:{}.", auditMap.toString()); @@ -330,6 +283,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, /** * Build audit map for RenameKeys request. + * * @param volumeName * @param bucketName * @param renameKeys @@ -337,17 +291,18 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, * @return */ private Map buildAuditMap(String volumeName, - String bucketName, List renameKeys, - List unRenameKeys) { + String bucketName, + List renameKeys, + List unRenameKeys) { Map renameKeysMap = new HashMap<>(); Map unRenameKeysMap = new HashMap<>(); Map auditMap = new HashMap<>(); - for(OmRenameKeyInfo keyInfo : renameKeys) { + for (OmRenameKeyInfo keyInfo : renameKeys) { renameKeysMap.put(keyInfo.getFromKeyName(), keyInfo.getNewKeyInfo().getKeyName()); } - for(RenameKeyArgs keyArgs : unRenameKeys) { + for (RenameKeyArgs keyArgs : unRenameKeys) { unRenameKeysMap.put(keyArgs.getFromKeyName(), keyArgs.getToKeyName()); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java index 8e23fad008d4..f3b796ab1ac1 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java @@ -18,7 +18,6 @@ package org.apache.hadoop.ozone.om.response.key; -import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.utils.db.BatchOperation; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OmRenameKeyInfo; @@ -68,32 +67,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, fromKeyName = omRenameKeyInfo.getFromKeyName(); OmKeyInfo newKeyInfo = omRenameKeyInfo.getNewKeyInfo(); toKeyName = newKeyInfo.getKeyName(); - // If toKeyName is null, then we need to only delete the fromKeyName - // from KeyTable. This is the case of replay where toKey exists but - // fromKey has not been deleted. - if (deleteFromKeyOnly()) { - omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, - omMetadataManager - .getOzoneKey(volumeName, bucketName, fromKeyName)); - } else if (createToKeyAndDeleteFromKey()) { - omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, - omMetadataManager - .getOzoneKey(volumeName, bucketName, fromKeyName)); - omMetadataManager.getKeyTable().putWithBatch(batchOperation, - omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName), - newKeyInfo); - } - } - } + omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, + omMetadataManager + .getOzoneKey(volumeName, bucketName, fromKeyName)); + omMetadataManager.getKeyTable().putWithBatch(batchOperation, + omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName), + newKeyInfo); - @VisibleForTesting - public boolean deleteFromKeyOnly() { - return toKeyName == null && fromKeyName != null; + } } - @VisibleForTesting - public boolean createToKeyAndDeleteFromKey() { - return toKeyName != null && !toKeyName.equals(fromKeyName); - } } From 1199843a6b92bb346141df10ff95e1bfc0cfcd1c Mon Sep 17 00:00:00 2001 From: captainzmc Date: Mon, 20 Jul 2020 15:06:09 +0800 Subject: [PATCH 08/15] fix review issues. --- ...ManagerProtocolClientSideTranslatorPB.java | 4 +- .../rpc/TestOzoneRpcClientAbstract.java | 100 +++++++++++------- .../apache/hadoop/ozone/om/OzoneManager.java | 5 +- .../om/request/key/OMKeysRenameRequest.java | 21 +--- .../key/TestOMKeysRenameResponse.java | 1 - 5 files changed, 67 insertions(+), 64 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 3d93ad5ecb14..99e554928c00 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -140,7 +140,6 @@ import org.apache.hadoop.ozone.security.proto.SecurityProtos.GetDelegationTokenRequestProto; import org.apache.hadoop.ozone.security.proto.SecurityProtos.RenewDelegationTokenRequestProto; import org.apache.hadoop.security.token.Token; -import org.apache.hadoop.util.Time; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -688,8 +687,7 @@ public void renameKeys(Map keyMap) throws IOException { KeyArgs keyArgs = KeyArgs.newBuilder() .setVolumeName(args.getVolumeName()) .setBucketName(args.getBucketName()) - .setKeyName(args.getKeyName()) - .setModificationTime(Time.now()).build(); + .setKeyName(args.getKeyName()).build(); reqKey.setKeyArgs(keyArgs); reqKey.setToKeyName(entry.getValue()); renameKeyRequestList.add(reqKey.build()); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java index 9512cc079fc0..45d07b097a38 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java @@ -124,7 +124,6 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import org.junit.Ignore; import org.junit.Test; /** @@ -1221,13 +1220,7 @@ public void testRenameKey() OzoneVolume volume = store.getVolume(volumeName); volume.createBucket(bucketName); OzoneBucket bucket = volume.getBucket(bucketName); - OzoneOutputStream out = bucket.createKey(fromKeyName, - value.getBytes().length, STAND_ALONE, - ONE, new HashMap<>()); - out.write(value.getBytes()); - out.close(); - OzoneKey key = bucket.getKey(fromKeyName); - Assert.assertEquals(fromKeyName, key.getName()); + createTestKey(bucket, fromKeyName, value); // Rename to empty string should fail. OMException oe = null; @@ -1250,7 +1243,7 @@ public void testRenameKey() } Assert.assertEquals(KEY_NOT_FOUND, oe.getResult()); - key = bucket.getKey(toKeyName); + OzoneKey key = bucket.getKey(toKeyName); Assert.assertEquals(toKeyName, key.getName()); } @@ -1269,55 +1262,58 @@ public void testKeysRename() throws Exception { OzoneVolume volume = store.getVolume(volumeName); volume.createBucket(bucketName); OzoneBucket bucket = volume.getBucket(bucketName); - OzoneOutputStream out1 = bucket.createKey(keyName1, - value.getBytes().length, STAND_ALONE, - ONE, new HashMap<>()); - out1.write(value.getBytes()); - out1.close(); - OzoneOutputStream out2 = bucket.createKey(keyName2, - value.getBytes().length, STAND_ALONE, - ONE, new HashMap<>()); - out2.write(value.getBytes()); - out2.close(); - OzoneKey key1 = bucket.getKey(keyName1); - OzoneKey key2 = bucket.getKey(keyName2); - Assert.assertEquals(keyName1, key1.getName()); - Assert.assertEquals(keyName2, key2.getName()); + createTestKey(bucket, keyName1, value); + createTestKey(bucket, keyName2, value); Map keyMap = new HashMap(); keyMap.put(keyName1, newKeyName1); keyMap.put(keyName2, newKeyName2); bucket.renameKeys(keyMap); + // new key should exist Assert.assertEquals(newKeyName1, bucket.getKey(newKeyName1).getName()); Assert.assertEquals(newKeyName2, bucket.getKey(newKeyName2).getName()); // old key should not exist - try { - bucket.getKey(keyName1); - } catch (OMException ex) { - Assert.assertEquals(KEY_NOT_FOUND, ex.getResult()); - } - try { - bucket.getKey(keyName2); - } catch (OMException ex) { - Assert.assertEquals(KEY_NOT_FOUND, ex.getResult()); - } + assertKeyRenamedEx(bucket, keyName1); + assertKeyRenamedEx(bucket, keyName2); + } + + @Test + public void testKeysRenameFail() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + String keyName1 = "dir/file1"; + String keyName2 = "dir/file2"; + + String newKeyName1 = "dir/key1"; + String newKeyName2 = "dir/key2"; + + String value = "sample value"; + store.createVolume(volumeName); + OzoneVolume volume = store.getVolume(volumeName); + volume.createBucket(bucketName); + OzoneBucket bucket = volume.getBucket(bucketName); + + // Create only keyName1 to test the partial failure of renameKeys. + createTestKey(bucket, keyName1, value); + + Map keyMap = new HashMap(); + keyMap.put(keyName1, newKeyName1); + keyMap.put(keyName2, newKeyName2); - // rename nonexistent key - Map keyMap1 = new HashMap(); - keyMap1.put(keyName1, keyName2); - keyMap1.put(newKeyName2, keyName2); try { - bucket.renameKeys(keyMap1); + bucket.renameKeys(keyMap); } catch (OMException ex) { Assert.assertEquals(PARTIAL_RENAME, ex.getResult()); } + + // newKeyName1 should exist + Assert.assertEquals(newKeyName1, bucket.getKey(newKeyName1).getName()); + // newKeyName2 should not exist + assertKeyRenamedEx(bucket, keyName2); } - // Listing all volumes in the cluster feature has to be fixed after HDDS-357. - // TODO: fix this - @Ignore @Test public void testListVolume() throws IOException { String volBase = "vol-" + RandomStringUtils.randomNumeric(3); @@ -2753,6 +2749,28 @@ private void completeMultipartUpload(OzoneBucket bucket, String keyName, Assert.assertNotNull(omMultipartUploadCompleteInfo.getHash()); } + private void createTestKey(OzoneBucket bucket, String keyName, + String keyValue) throws IOException { + OzoneOutputStream out = bucket.createKey(keyName, + keyValue.getBytes().length, STAND_ALONE, + ONE, new HashMap<>()); + out.write(keyValue.getBytes()); + out.close(); + OzoneKey key = bucket.getKey(keyName); + Assert.assertEquals(keyName, key.getName()); + } + + private void assertKeyRenamedEx(OzoneBucket bucket, String keyName) + throws Exception { + OMException oe = null; + try { + bucket.getKey(keyName); + } catch (OMException e) { + oe = e; + } + Assert.assertEquals(KEY_NOT_FOUND, oe.getResult()); + } + /** * Tests GDPR encryption/decryption. * 1. Create GDPR Enabled bucket. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 4c89798fd722..fef849d87b8d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -181,7 +181,6 @@ import com.google.common.base.Preconditions; import com.google.protobuf.BlockingService; import com.google.protobuf.ProtocolMessageEnum; -import org.apache.commons.lang3.NotImplementedException; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; @@ -2227,8 +2226,8 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { @Override public void renameKeys(Map omKeyArgsMap) throws IOException { - throw new NotImplementedException("OzoneManager does not require this to " + - "be implemented. As write requests use a new approach"); + throw new UnsupportedOperationException("OzoneManager does not require " + + "this to be implemented. As write requests use a new approach"); } @Override diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 339b1641804c..854c64c6a87d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -82,7 +82,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { List renameKeyList = new ArrayList<>(); for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) { - // Set modification time. + // Set modification time in preExecute. KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder() .setModificationTime(Time.now()); renameKey.toBuilder().setKeyArgs(newKeyArgs); @@ -101,7 +101,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest(); OMClientResponse omClientResponse = null; - // fromKeyName -> toKeyNmae + // fromKeyName -> toKeyName List unRenamedKeys = new ArrayList<>(); List renameKeyInfoList = new ArrayList<>(); @@ -133,7 +133,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, .getRenameKeyRequestList()) { OzoneManagerProtocolProtos.KeyArgs keyArgs = renameKeyRequest.getKeyArgs(); - + auditMap = buildAuditMap(volumeName, bucketName, renameKeyInfoList, + unRenamedKeys); + keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap); volumeName = keyArgs.getVolumeName(); bucketName = keyArgs.getBucketName(); fromKeyName = keyArgs.getKeyName(); @@ -144,17 +146,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, .setVolumeName(volumeName).setBucketName(bucketName) .setFromKeyName(fromKeyName).setToKeyName(toKeyName).build(); - try { - // Validate bucket and volume exists or not. - validateBucketAndVolume(omMetadataManager, volumeName, bucketName); - } catch (Exception ex) { - renameStatus = false; - unRenamedKeys.add(renameKeyArgs); - LOG.error("Validate bucket and volume exists failed" + - "volumeName {} bucketName {}", volumeName, bucketName, ex); - continue; - } - if (toKeyName.length() == 0 || fromKeyName.length() == 0) { renameStatus = false; unRenamedKeys.add(renameKeyArgs); @@ -259,8 +250,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } } - auditMap = buildAuditMap(volumeName, bucketName, renameKeyInfoList, - unRenamedKeys); auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEYS, auditMap, exception, getOmRequest().getUserInfo())); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java index 18223e61b9ae..8f2b32387706 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java @@ -103,7 +103,6 @@ public void testKeysRenameResponseFail() throws Exception { Assert.assertFalse(omMetadataManager.getKeyTable().isExist(toKey)); } - } private void createPreRequisities() throws Exception { From 2795f76aafe92e7067de9ddfe4f03cdee090d946 Mon Sep 17 00:00:00 2001 From: captainzmc Date: Tue, 21 Jul 2020 14:04:53 +0800 Subject: [PATCH 09/15] fix auditMap update issues. --- .../om/request/key/OMKeysRenameRequest.java | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 854c64c6a87d..f57f5a2ee572 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -51,6 +51,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -69,6 +70,8 @@ public class OMKeysRenameRequest extends OMKeyRequest { private static final Logger LOG = LoggerFactory.getLogger(OMKeysRenameRequest.class); + private String volumeName = null; + private String bucketName = null; public OMKeysRenameRequest(OMRequest omRequest) { super(omRequest); @@ -87,6 +90,12 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { .setModificationTime(Time.now()); renameKey.toBuilder().setKeyArgs(newKeyArgs); renameKeyList.add(renameKey); + if (volumeName == null) { + volumeName = renameKey.getKeyArgs().getVolumeName(); + } + if (bucketName == null) { + bucketName = renameKey.getKeyArgs().getBucketName(); + } } RenameKeysRequest renameKeysRequest = RenameKeysRequest .newBuilder().addAllRenameKeyRequest(renameKeyList).build(); @@ -111,7 +120,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, AuditLogger auditLogger = ozoneManager.getAuditLogger(); - OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder( getOmRequest()); @@ -120,27 +128,25 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OmKeyInfo fromKeyValue = null; Result result = null; - Map auditMap = null; - RenameKeyRequest renameRequest = null; - String volumeName = null; - String bucketName = null; + Map auditMap = new LinkedHashMap<>(); + auditMap.put(VOLUME, volumeName); + auditMap.put(BUCKET, bucketName); String fromKeyName = null; String toKeyName = null; boolean acquiredLock = false; boolean renameStatus = true; + try { for (RenameKeyRequest renameKeyRequest : renameKeysRequest .getRenameKeyRequestList()) { OzoneManagerProtocolProtos.KeyArgs keyArgs = renameKeyRequest.getKeyArgs(); - auditMap = buildAuditMap(volumeName, bucketName, renameKeyInfoList, - unRenamedKeys); + keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap); volumeName = keyArgs.getVolumeName(); bucketName = keyArgs.getBucketName(); fromKeyName = keyArgs.getKeyName(); toKeyName = renameKeyRequest.getToKeyName(); - renameRequest = renameKeyRequest; RenameKeyArgs renameKeyArgs = RenameKeyArgs.newBuilder() .setVolumeName(volumeName).setBucketName(bucketName) @@ -250,6 +256,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } } + auditMap = buildAuditMap(auditMap, renameKeyInfoList, unRenamedKeys); auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEYS, auditMap, exception, getOmRequest().getUserInfo())); @@ -263,8 +270,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, LOG.error("Rename keys failed for auditMap:{}.", auditMap.toString()); break; default: - LOG.error("Unrecognized Result for OMKeyRenameRequest: {}", - renameRequest); + LOG.error("Unrecognized Result for OMKeysRenameRequest: {}", + renameKeysRequest); } return omClientResponse; @@ -273,19 +280,15 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, /** * Build audit map for RenameKeys request. * - * @param volumeName - * @param bucketName * @param renameKeys * @param unRenameKeys * @return */ - private Map buildAuditMap(String volumeName, - String bucketName, + private Map buildAuditMap(Map auditMap, List renameKeys, List unRenameKeys) { Map renameKeysMap = new HashMap<>(); Map unRenameKeysMap = new HashMap<>(); - Map auditMap = new HashMap<>(); for (OmRenameKeyInfo keyInfo : renameKeys) { renameKeysMap.put(keyInfo.getFromKeyName(), @@ -295,8 +298,6 @@ private Map buildAuditMap(String volumeName, unRenameKeysMap.put(keyArgs.getFromKeyName(), keyArgs.getToKeyName()); } - auditMap.put(VOLUME, volumeName); - auditMap.put(BUCKET, bucketName); auditMap.put(RENAMED_KEYS_MAP, renameKeysMap.toString()); auditMap.put(UNRENAMED_KEYS_MAP, unRenameKeysMap.toString()); return auditMap; From ac6029b8b57795b4795e47a707328c6793e1116d Mon Sep 17 00:00:00 2001 From: captainzmc Date: Wed, 29 Jul 2020 21:35:39 +0800 Subject: [PATCH 10/15] resolve conflict. --- .../java/org/apache/hadoop/ozone/om/exceptions/OMException.java | 2 +- hadoop-ozone/interface-client/src/main/resources/proto.lock | 2 +- .../src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java index ce0001fcd04e..e08dccb6a5a1 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java @@ -226,7 +226,7 @@ public enum ResultCodes { PARTIAL_DELETE, DETECTED_LOOP_IN_BUCKET_LINKS, - + NOT_SUPPORTED_OPERATION, PARTIAL_RENAME diff --git a/hadoop-ozone/interface-client/src/main/resources/proto.lock b/hadoop-ozone/interface-client/src/main/resources/proto.lock index 1a2e6ca1b0f7..20abf33e8155 100644 --- a/hadoop-ozone/interface-client/src/main/resources/proto.lock +++ b/hadoop-ozone/interface-client/src/main/resources/proto.lock @@ -426,7 +426,7 @@ }, { "name": "PARTIAL_RENAME", - "integer": 64 + "integer": 65 } ] }, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index fef849d87b8d..833ae5860149 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -48,7 +48,6 @@ import java.util.concurrent.TimeUnit; import com.google.common.base.Optional; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.conf.StorageUnit; import org.apache.hadoop.crypto.key.KeyProvider; import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension; From e1070e962c0cace7a49a8da4f8e6a0f7c0536c31 Mon Sep 17 00:00:00 2001 From: captainzmc Date: Thu, 30 Jul 2020 10:25:41 +0800 Subject: [PATCH 11/15] Resolve bucket links only once. --- .../src/main/resources/proto.lock | 2 +- .../om/request/key/OMKeysRenameRequest.java | 21 +++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/hadoop-hdds/interface-client/src/main/resources/proto.lock b/hadoop-hdds/interface-client/src/main/resources/proto.lock index 5ba0625f33fb..b27896c655e3 100644 --- a/hadoop-hdds/interface-client/src/main/resources/proto.lock +++ b/hadoop-hdds/interface-client/src/main/resources/proto.lock @@ -1935,4 +1935,4 @@ } } ] -} \ No newline at end of file +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index f57f5a2ee572..088498c77269 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -90,12 +90,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { .setModificationTime(Time.now()); renameKey.toBuilder().setKeyArgs(newKeyArgs); renameKeyList.add(renameKey); - if (volumeName == null) { - volumeName = renameKey.getKeyArgs().getVolumeName(); - } - if (bucketName == null) { - bucketName = renameKey.getKeyArgs().getBucketName(); - } + } RenameKeysRequest renameKeysRequest = RenameKeysRequest .newBuilder().addAllRenameKeyRequest(renameKeyList).build(); @@ -129,8 +124,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Result result = null; Map auditMap = new LinkedHashMap<>(); - auditMap.put(VOLUME, volumeName); - auditMap.put(BUCKET, bucketName); String fromKeyName = null; String toKeyName = null; boolean acquiredLock = false; @@ -142,9 +135,15 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OzoneManagerProtocolProtos.KeyArgs keyArgs = renameKeyRequest.getKeyArgs(); - keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap); - volumeName = keyArgs.getVolumeName(); - bucketName = keyArgs.getBucketName(); + // resolve Bucket Link at the first time. + if (bucketName == null) { + keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap); + volumeName = keyArgs.getVolumeName(); + bucketName = keyArgs.getBucketName(); + auditMap.put(VOLUME, volumeName); + auditMap.put(BUCKET, bucketName); + } + fromKeyName = keyArgs.getKeyName(); toKeyName = renameKeyRequest.getToKeyName(); From 9f5fda0f24ecef4f5014c1a85839c542ee579a4d Mon Sep 17 00:00:00 2001 From: captainzmc Date: Thu, 30 Jul 2020 20:12:54 +0800 Subject: [PATCH 12/15] doesn't use KeyArgs and fix other review issues --- .../hadoop/ozone/client/rpc/RpcClient.java | 15 +- .../hadoop/ozone/om/helpers/OmRenameKeys.java | 59 ++++++++ .../om/protocol/OzoneManagerProtocol.java | 7 +- ...ManagerProtocolClientSideTranslatorPB.java | 36 +++-- .../src/main/proto/OmClientProtocol.proto | 16 ++- .../src/main/resources/proto.lock | 70 ---------- .../hadoop/ozone/om/OmRenameKeyInfo.java | 47 ------- .../apache/hadoop/ozone/om/OzoneManager.java | 3 +- .../om/request/key/OMKeysRenameRequest.java | 129 +++++++----------- .../om/response/key/OMKeysRenameResponse.java | 27 ++-- .../request/key/TestOMKeysRenameRequest.java | 40 +++--- .../key/TestOMKeysRenameResponse.java | 19 +-- 12 files changed, 199 insertions(+), 269 deletions(-) create mode 100644 hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmRenameKeys.java delete mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index f54b2b7ee79e..3f984d279161 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -27,7 +27,6 @@ import java.security.SecureRandom; import java.util.ArrayList; import java.util.Arrays; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; @@ -86,6 +85,7 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadList; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadListParts; import org.apache.hadoop.ozone.om.helpers.OmPartInfo; +import org.apache.hadoop.ozone.om.helpers.OmRenameKeys; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.OpenKeySession; import org.apache.hadoop.ozone.om.helpers.OzoneAclUtil; @@ -767,16 +767,9 @@ public void renameKeys(String volumeName, String bucketName, verifyVolumeName(volumeName); verifyBucketName(bucketName); HddsClientUtils.checkNotNull(keyMap); - Map keyArgsMap = new HashMap<>(); - for (Map.Entry< String, String > entry : keyMap.entrySet()) { - OmKeyArgs keyArgs = new OmKeyArgs.Builder() - .setVolumeName(volumeName) - .setBucketName(bucketName) - .setKeyName(entry.getKey()) - .build(); - keyArgsMap.put(keyArgs, entry.getValue()); - } - ozoneManagerClient.renameKeys(keyArgsMap); + OmRenameKeys omRenameKeys = + new OmRenameKeys(volumeName, bucketName, keyMap, null); + ozoneManagerClient.renameKeys(omRenameKeys); } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmRenameKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmRenameKeys.java new file mode 100644 index 000000000000..d550817b6a25 --- /dev/null +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmRenameKeys.java @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om.helpers; + +import java.util.HashMap; +import java.util.Map; + +/** + * This class is used for rename keys. + */ +public class OmRenameKeys { + + private String volume; + private String bucket; + private Map fromAndToKey = new HashMap<>(); + private Map fromKeyAndToKeyInfo = new HashMap<>(); + + public OmRenameKeys(String volume, String bucket, + Map fromAndToKey, + Map fromKeyAndToKeyInfo) { + this.volume = volume; + this.bucket = bucket; + this.fromAndToKey = fromAndToKey; + this.fromKeyAndToKeyInfo = fromKeyAndToKeyInfo; + } + + public String getVolume() { + return volume; + } + + public String getBucket() { + return bucket; + } + + public Map getFromAndToKey() { + return fromAndToKey; + } + + public Map getFromKeyAndToKeyInfo() { + return fromKeyAndToKeyInfo; + } + +} diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index 20957b615300..267ac89be03f 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -21,7 +21,6 @@ import java.io.Closeable; import java.io.IOException; import java.util.List; -import java.util.Map; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; import org.apache.hadoop.ozone.OzoneAcl; @@ -40,6 +39,7 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadCompleteList; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadList; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadListParts; +import org.apache.hadoop.ozone.om.helpers.OmRenameKeys; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.OpenKeySession; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; @@ -220,10 +220,11 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID, /** * Rename existing keys within a bucket. - * @param keyMap The key is original key OmKeyArgs and value is new key name. + * @param omRenameKeys Includes volume, bucket, and fromKey toKey name map + * and fromKey name toKey info Map. * @throws IOException */ - void renameKeys(Map keyMap) throws IOException; + void renameKeys(OmRenameKeys omRenameKeys) throws IOException; /** * Deletes an existing key. diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 99e554928c00..a6ea0423bcf0 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -45,6 +45,7 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadCompleteList; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadList; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadListParts; +import org.apache.hadoop.ozone.om.helpers.OmRenameKeys; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.OpenKeySession; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; @@ -122,6 +123,8 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RecoverTrashResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RemoveAclRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RemoveAclResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysMap; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenewDelegationTokenResponseProto; @@ -678,24 +681,27 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { } @Override - public void renameKeys(Map keyMap) throws IOException { - RenameKeysRequest.Builder reqKeys = RenameKeysRequest.newBuilder(); - List renameKeyRequestList = new ArrayList<>(); - for (Map.Entry< OmKeyArgs, String> entry : keyMap.entrySet()) { - RenameKeyRequest.Builder reqKey = RenameKeyRequest.newBuilder(); - OmKeyArgs args = entry.getKey(); - KeyArgs keyArgs = KeyArgs.newBuilder() - .setVolumeName(args.getVolumeName()) - .setBucketName(args.getBucketName()) - .setKeyName(args.getKeyName()).build(); - reqKey.setKeyArgs(keyArgs); - reqKey.setToKeyName(entry.getValue()); - renameKeyRequestList.add(reqKey.build()); + public void renameKeys(OmRenameKeys omRenameKeys) throws IOException { + + List renameKeyList = new ArrayList<>(); + for (Map.Entry< String, String> entry : + omRenameKeys.getFromAndToKey().entrySet()) { + RenameKeysMap.Builder renameKey = RenameKeysMap.newBuilder() + .setFromKeyName(entry.getKey()) + .setToKeyName(entry.getValue()); + renameKeyList.add(renameKey.build()); } - reqKeys.addAllRenameKeyRequest(renameKeyRequestList); + + RenameKeysArgs.Builder renameKeyArgs = RenameKeysArgs.newBuilder() + .setVolumeName(omRenameKeys.getVolume()) + .setBucketName(omRenameKeys.getBucket()) + .addAllRenameKeysMap(renameKeyList); + + RenameKeysRequest.Builder reqKeys = RenameKeysRequest.newBuilder() + .setRenameKeysArgs(renameKeyArgs.build()); OMRequest omRequest = createOMRequest(Type.RenameKeys) - .setRenameKeysRequest(reqKeys) + .setRenameKeysRequest(reqKeys.build()) .build(); handleError(submitRequest(omRequest)); diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index e1e062292937..c6e2949122c1 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -845,19 +845,23 @@ message LookupKeyResponse { optional uint64 openVersion = 4; } -message RenameKeysRequest{ - repeated RenameKeyRequest renameKeyRequest = 1; +message RenameKeysRequest { + required RenameKeysArgs renameKeysArgs = 1; } -message RenameKeyArgs{ +message RenameKeysArgs { required string volumeName = 1; required string bucketName = 2; - optional string fromKeyName = 3; - optional string toKeyName = 4; + repeated RenameKeysMap renameKeysMap = 3; +} + +message RenameKeysMap { + required string fromKeyName = 1; + required string toKeyName = 2; } message RenameKeysResponse{ - repeated RenameKeyArgs unRenamedKeys = 1; + repeated RenameKeysMap unRenamedKeys = 1; optional bool status = 2; } diff --git a/hadoop-ozone/interface-client/src/main/resources/proto.lock b/hadoop-ozone/interface-client/src/main/resources/proto.lock index 20abf33e8155..bef3a0df1e8d 100644 --- a/hadoop-ozone/interface-client/src/main/resources/proto.lock +++ b/hadoop-ozone/interface-client/src/main/resources/proto.lock @@ -83,10 +83,6 @@ "name": "DeleteKeys", "integer": 38 }, - { - "name": "RenameKeys", - "integer": 39 - }, { "name": "InitiateMultiPartUpload", "integer": 45 @@ -423,10 +419,6 @@ { "name": "PARTIAL_DELETE", "integer": 62 - }, - { - "name": "PARTIAL_RENAME", - "integer": 65 } ] }, @@ -719,11 +711,6 @@ "name": "deleteKeysRequest", "type": "DeleteKeysRequest" }, - { - "id": 39, - "name": "renameKeysRequest", - "type": "RenameKeysRequest" - }, { "id": 45, "name": "initiateMultiPartUploadRequest", @@ -995,11 +982,6 @@ "name": "deleteKeysResponse", "type": "DeleteKeysResponse" }, - { - "id": 39, - "name": "renameKeysResponse", - "type": "RenameKeysResponse" - }, { "id": 45, "name": "initiateMultiPartUploadResponse", @@ -2419,58 +2401,6 @@ } ] }, - { - "name": "RenameKeysRequest", - "fields": [ - { - "id": 1, - "name": "renameKeyRequest", - "type": "RenameKeyRequest", - "is_repeated": true - } - ] - }, - { - "name": "RenameKeyArgs", - "fields": [ - { - "id": 1, - "name": "volumeName", - "type": "string" - }, - { - "id": 2, - "name": "bucketName", - "type": "string" - }, - { - "id": 3, - "name": "fromKeyName", - "type": "string" - }, - { - "id": 4, - "name": "toKeyName", - "type": "string" - } - ] - }, - { - "name": "RenameKeysResponse", - "fields": [ - { - "id": 1, - "name": "unRenamedKeys", - "type": "RenameKeyArgs", - "is_repeated": true - }, - { - "id": 2, - "name": "status", - "type": "bool" - } - ] - }, { "name": "RenameKeyRequest", "fields": [ diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java deleted file mode 100644 index 8b5ed1efced5..000000000000 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmRenameKeyInfo.java +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.hadoop.ozone.om; - -import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; - -import javax.annotation.Nonnull; - -/** - * This class is used for rename keys. - */ -public class OmRenameKeyInfo { - - private OmKeyInfo newKeyInfo; - private String fromKeyName; - - public OmRenameKeyInfo(String fromKeyName, - @Nonnull OmKeyInfo renameKeyInfo) { - this.fromKeyName = fromKeyName; - this.newKeyInfo = renameKeyInfo; - } - - public String getFromKeyName() { - return fromKeyName; - } - - public OmKeyInfo getNewKeyInfo() { - return newKeyInfo; - } - -} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 833ae5860149..85081aecf82f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -124,6 +124,7 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadCompleteList; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadList; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadListParts; +import org.apache.hadoop.ozone.om.helpers.OmRenameKeys; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.OpenKeySession; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; @@ -2223,7 +2224,7 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { @Override - public void renameKeys(Map omKeyArgsMap) + public void renameKeys(OmRenameKeys omRenameKeys) throws IOException { throw new UnsupportedOperationException("OzoneManager does not require " + "this to be implemented. As write requests use a new approach"); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 088498c77269..fa7411fba987 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -19,7 +19,7 @@ package org.apache.hadoop.ozone.om.request.key; import com.google.common.base.Optional; -import com.google.common.base.Preconditions; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; @@ -27,19 +27,18 @@ import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; -import org.apache.hadoop.ozone.om.OmRenameKeyInfo; +import org.apache.hadoop.ozone.om.ResolvedBucket; +import org.apache.hadoop.ozone.om.helpers.OmRenameKeys; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysMap; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; @@ -57,10 +56,8 @@ import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME; -import static org.apache.hadoop.ozone.OzoneConsts.BUCKET; import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP; import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP; -import static org.apache.hadoop.ozone.OzoneConsts.VOLUME; import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; /** @@ -70,46 +67,28 @@ public class OMKeysRenameRequest extends OMKeyRequest { private static final Logger LOG = LoggerFactory.getLogger(OMKeysRenameRequest.class); - private String volumeName = null; - private String bucketName = null; public OMKeysRenameRequest(OMRequest omRequest) { super(omRequest); } - @Override - public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { - - RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest(); - Preconditions.checkNotNull(renameKeys); - - List renameKeyList = new ArrayList<>(); - for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) { - // Set modification time in preExecute. - KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder() - .setModificationTime(Time.now()); - renameKey.toBuilder().setKeyArgs(newKeyArgs); - renameKeyList.add(renameKey); - - } - RenameKeysRequest renameKeysRequest = RenameKeysRequest - .newBuilder().addAllRenameKeyRequest(renameKeyList).build(); - return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest) - .setUserInfo(getUserInfo()).build(); - } - @Override @SuppressWarnings("methodlength") public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) { RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest(); + RenameKeysArgs renameKeysArgs = renameKeysRequest.getRenameKeysArgs(); + String volumeName = renameKeysArgs.getVolumeName(); + String bucketName = renameKeysArgs.getBucketName(); OMClientResponse omClientResponse = null; - // fromKeyName -> toKeyName - List unRenamedKeys = new ArrayList<>(); - List renameKeyInfoList = new ArrayList<>(); + List unRenamedKeys = new ArrayList<>(); + // fromKeyName -> toKeyName + Map renamedKeys = new HashMap<>(); + + Map fromKeyAndToKeyInfo = new HashMap<>(); OMMetrics omMetrics = ozoneManager.getMetrics(); omMetrics.incNumKeyRenames(); @@ -121,7 +100,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager(); IOException exception = null; OmKeyInfo fromKeyValue = null; - Result result = null; Map auditMap = new LinkedHashMap<>(); String fromKeyName = null; @@ -130,30 +108,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, boolean renameStatus = true; try { - for (RenameKeyRequest renameKeyRequest : renameKeysRequest - .getRenameKeyRequestList()) { - OzoneManagerProtocolProtos.KeyArgs keyArgs = - renameKeyRequest.getKeyArgs(); - - // resolve Bucket Link at the first time. - if (bucketName == null) { - keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap); - volumeName = keyArgs.getVolumeName(); - bucketName = keyArgs.getBucketName(); - auditMap.put(VOLUME, volumeName); - auditMap.put(BUCKET, bucketName); - } + ResolvedBucket bucket = ozoneManager.resolveBucketLink( + Pair.of(volumeName, bucketName)); + bucket.audit(auditMap); + volumeName = bucket.realVolume(); + bucketName = bucket.realBucket(); - fromKeyName = keyArgs.getKeyName(); - toKeyName = renameKeyRequest.getToKeyName(); + for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) { - RenameKeyArgs renameKeyArgs = RenameKeyArgs.newBuilder() - .setVolumeName(volumeName).setBucketName(bucketName) - .setFromKeyName(fromKeyName).setToKeyName(toKeyName).build(); + fromKeyName = renameKey.getFromKeyName(); + toKeyName = renameKey.getToKeyName(); + RenameKeysMap.Builder unRenameKey = RenameKeysMap.newBuilder(); if (toKeyName.length() == 0 || fromKeyName.length() == 0) { renameStatus = false; - unRenamedKeys.add(renameKeyArgs); + unRenamedKeys.add( + unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName) + .build()); LOG.error("Key name is empty fromKeyName {} toKeyName {}", fromKeyName, toKeyName); continue; @@ -168,7 +139,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY); } catch (Exception ex) { renameStatus = false; - unRenamedKeys.add(renameKeyArgs); + unRenamedKeys.add( + unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName) + .build()); LOG.error("Acl check failed for fromKeyName {} toKeyName {}", fromKeyName, toKeyName, ex); continue; @@ -184,7 +157,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (toKeyValue != null) { renameStatus = false; - unRenamedKeys.add(renameKeyArgs); + unRenamedKeys.add( + unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName) + .build()); LOG.error("Received a request name of new key {} already exists", toKeyName); } @@ -193,7 +168,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, fromKeyValue = omMetadataManager.getKeyTable().get(fromKey); if (fromKeyValue == null) { renameStatus = false; - unRenamedKeys.add(renameKeyArgs); + unRenamedKeys.add( + unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName) + .build()); LOG.error("Received a request to rename a Key does not exist {}", fromKey); continue; @@ -204,7 +181,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, fromKeyValue.setKeyName(toKeyName); //Set modification time - fromKeyValue.setModificationTime(keyArgs.getModificationTime()); + fromKeyValue.setModificationTime(Time.now()); acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, @@ -217,24 +194,27 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, new CacheValue<>(Optional.absent(), trxnLogIndex)); keyTable.addCacheEntry(new CacheKey<>(toKey), new CacheValue<>(Optional.of(fromKeyValue), trxnLogIndex)); - renameKeyInfoList - .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue)); - + renamedKeys.put(fromKeyName, toKeyName); + fromKeyAndToKeyInfo.put(fromKeyName, fromKeyValue); if (acquiredLock) { omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName, bucketName); acquiredLock = false; } } + acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName, bucketName); + OmRenameKeys newOmRenameKeys = + new OmRenameKeys(volumeName, bucketName, null, fromKeyAndToKeyInfo); omClientResponse = new OMKeysRenameResponse(omResponse .setRenameKeysResponse(RenameKeysResponse.newBuilder() - .setStatus(renameStatus).addAllUnRenamedKeys(unRenamedKeys)) + .setStatus(renameStatus) + .addAllUnRenamedKeys(unRenamedKeys)) .setStatus(renameStatus ? OK : PARTIAL_RENAME) .setSuccess(renameStatus).build(), - renameKeyInfoList); + newOmRenameKeys); result = Result.SUCCESS; } catch (IOException ex) { @@ -255,7 +235,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } } - auditMap = buildAuditMap(auditMap, renameKeyInfoList, unRenamedKeys); + auditMap = buildAuditMap(auditMap, renamedKeys, unRenamedKeys); auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEYS, auditMap, exception, getOmRequest().getUserInfo())); @@ -279,25 +259,20 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, /** * Build audit map for RenameKeys request. * - * @param renameKeys + * @param auditMap + * @param renamedKeys * @param unRenameKeys * @return */ private Map buildAuditMap(Map auditMap, - List renameKeys, - List unRenameKeys) { - Map renameKeysMap = new HashMap<>(); + Map renamedKeys, + List unRenameKeys) { Map unRenameKeysMap = new HashMap<>(); - - for (OmRenameKeyInfo keyInfo : renameKeys) { - renameKeysMap.put(keyInfo.getFromKeyName(), - keyInfo.getNewKeyInfo().getKeyName()); + for (RenameKeysMap renameKeysMap : unRenameKeys) { + unRenameKeysMap.put(renameKeysMap.getFromKeyName(), + renameKeysMap.getToKeyName()); } - for (RenameKeyArgs keyArgs : unRenameKeys) { - unRenameKeysMap.put(keyArgs.getFromKeyName(), keyArgs.getToKeyName()); - } - - auditMap.put(RENAMED_KEYS_MAP, renameKeysMap.toString()); + auditMap.put(RENAMED_KEYS_MAP, renamedKeys.toString()); auditMap.put(UNRENAMED_KEYS_MAP, unRenameKeysMap.toString()); return auditMap; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java index f3b796ab1ac1..a9ff7ada1bd7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java @@ -20,7 +20,7 @@ import org.apache.hadoop.hdds.utils.db.BatchOperation; import org.apache.hadoop.ozone.om.OMMetadataManager; -import org.apache.hadoop.ozone.om.OmRenameKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OmRenameKeys; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.response.CleanupTableInfo; import org.apache.hadoop.ozone.om.response.OMClientResponse; @@ -28,7 +28,7 @@ import javax.annotation.Nonnull; import java.io.IOException; -import java.util.List; +import java.util.Map; import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.KEY_TABLE; @@ -38,14 +38,12 @@ @CleanupTableInfo(cleanupTables = {KEY_TABLE}) public class OMKeysRenameResponse extends OMClientResponse { - private List renameKeyInfoList; - private String fromKeyName = null; - private String toKeyName = null; + private OmRenameKeys omRenameKeys; public OMKeysRenameResponse(@Nonnull OMResponse omResponse, - List renameKeyInfoList) { + OmRenameKeys omRenameKeys) { super(omResponse); - this.renameKeyInfoList = renameKeyInfoList; + this.omRenameKeys = omRenameKeys; } @@ -61,12 +59,14 @@ public OMKeysRenameResponse(@Nonnull OMResponse omResponse) { @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { - for (OmRenameKeyInfo omRenameKeyInfo : renameKeyInfoList) { - String volumeName = omRenameKeyInfo.getNewKeyInfo().getVolumeName(); - String bucketName = omRenameKeyInfo.getNewKeyInfo().getBucketName(); - fromKeyName = omRenameKeyInfo.getFromKeyName(); - OmKeyInfo newKeyInfo = omRenameKeyInfo.getNewKeyInfo(); - toKeyName = newKeyInfo.getKeyName(); + String volumeName = omRenameKeys.getVolume(); + String bucketName = omRenameKeys.getBucket(); + + for (Map.Entry< String, OmKeyInfo> entry : + omRenameKeys.getFromKeyAndToKeyInfo().entrySet()) { + String fromKeyName = entry.getKey(); + OmKeyInfo newKeyInfo = entry.getValue(); + String toKeyName = newKeyInfo.getKeyName(); omMetadataManager.getKeyTable().deleteWithBatch(batchOperation, omMetadataManager @@ -74,7 +74,6 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, omMetadataManager.getKeyTable().putWithBatch(batchOperation, omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName), newKeyInfo); - } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysRenameRequest.java index 93657a71268d..947590660a84 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysRenameRequest.java @@ -23,14 +23,15 @@ import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysMap; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest; import org.junit.Assert; import org.junit.Test; +import java.util.ArrayList; +import java.util.List; import java.util.UUID; /** @@ -102,7 +103,7 @@ public void testKeysRenameRequestFail() throws Exception { } // The key not rename should be in unRenamedKeys. - RenameKeyArgs unRenamedKeys = omKeysRenameResponse.getOMResponse() + RenameKeysMap unRenamedKeys = omKeysRenameResponse.getOMResponse() .getRenameKeysResponse().getUnRenamedKeys(0); Assert.assertEquals("testKey", unRenamedKeys.getFromKeyName()); } @@ -114,12 +115,12 @@ public void testKeysRenameRequestFail() throws Exception { */ private OMRequest createRenameKeyRequest(Boolean isIllegal) throws Exception { - RenameKeysRequest.Builder renameKeysReq = RenameKeysRequest.newBuilder(); - // Add volume, bucket and key entries to OM DB. TestOMRequestUtils.addVolumeAndBucketToDB(volumeName, bucketName, omMetadataManager); + List renameKeyList = new ArrayList<>(); + for (int i = 0; i < count; i++) { String key = parentDir.concat("/key" + i); String toKey = parentDir.concat("/newKey" + i); @@ -127,22 +128,29 @@ private OMRequest createRenameKeyRequest(Boolean isIllegal) throws Exception { parentDir.concat("/key" + i), HddsProtos.ReplicationType.RATIS, HddsProtos.ReplicationFactor.THREE, omMetadataManager); - KeyArgs keyArgs = KeyArgs.newBuilder().setKeyName(key) - .setVolumeName(volumeName).setBucketName(bucketName).build(); - RenameKeyRequest renameKeyRequest = RenameKeyRequest.newBuilder() - .setKeyArgs(keyArgs).setToKeyName(toKey).build(); - renameKeysReq.addRenameKeyRequest(renameKeyRequest); + RenameKeysMap.Builder renameKey = RenameKeysMap.newBuilder() + .setFromKeyName(key) + .setToKeyName(toKey); + renameKeyList.add(renameKey.build()); } + // Generating illegal data causes Rename Keys to fail. if (isIllegal) { - KeyArgs keyArgs = KeyArgs.newBuilder().setKeyName("testKey") - .setVolumeName(volumeName).setBucketName(bucketName).build(); - RenameKeyRequest renameKeyRequest = RenameKeyRequest.newBuilder() - .setKeyArgs(keyArgs).setToKeyName("testToKey").build(); - renameKeysReq.addRenameKeyRequest(renameKeyRequest); + RenameKeysMap.Builder renameKey = RenameKeysMap.newBuilder() + .setFromKeyName("testKey") + .setToKeyName("toKey"); + renameKeyList.add(renameKey.build()); } + RenameKeysArgs.Builder renameKeyArgs = RenameKeysArgs.newBuilder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + .addAllRenameKeysMap(renameKeyList); + + RenameKeysRequest.Builder renameKeysReq = RenameKeysRequest.newBuilder() + .setRenameKeysArgs(renameKeyArgs.build()); + return OMRequest.newBuilder() .setClientId(UUID.randomUUID().toString()) .setRenameKeysRequest(renameKeysReq.build()) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java index 8f2b32387706..a9db1b839758 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysRenameResponse.java @@ -18,7 +18,7 @@ package org.apache.hadoop.ozone.om.response.key; -import org.apache.hadoop.ozone.om.OmRenameKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OmRenameKeys; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; @@ -29,8 +29,8 @@ import org.junit.Assert; import org.junit.Test; -import java.util.ArrayList; -import java.util.List; +import java.util.HashMap; +import java.util.Map; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType.RATIS; @@ -39,7 +39,7 @@ * Tests OMKeyRenameResponse. */ public class TestOMKeysRenameResponse extends TestOMKeyResponse { - private List renameKeyInfoList; + private OmRenameKeys omRenameKeys; private int count = 10; private String parentDir = "/test"; @@ -53,7 +53,7 @@ public void testKeysRenameResponse() throws Exception { .setStatus(Status.OK).setCmdType(Type.RenameKeys).build(); OMKeysRenameResponse omKeysRenameResponse = new OMKeysRenameResponse( - omResponse, renameKeyInfoList); + omResponse, omRenameKeys); omKeysRenameResponse.addToDBBatch(omMetadataManager, batchOperation); @@ -86,7 +86,7 @@ public void testKeysRenameResponseFail() throws Exception { .build(); OMKeysRenameResponse omKeyRenameResponse = new OMKeysRenameResponse( - omResponse, renameKeyInfoList); + omResponse, omRenameKeys); omKeyRenameResponse.checkAndUpdateDB(omMetadataManager, batchOperation); @@ -107,10 +107,10 @@ public void testKeysRenameResponseFail() throws Exception { private void createPreRequisities() throws Exception { - renameKeyInfoList = new ArrayList<>(); // Add volume, bucket and key entries to OM DB. TestOMRequestUtils.addVolumeAndBucketToDB(volumeName, bucketName, omMetadataManager); + Map formAndToKeyInfo = new HashMap<>(); for (int i = 0; i < count; i++) { String key = parentDir.concat("/key" + i); @@ -122,9 +122,10 @@ private void createPreRequisities() throws Exception { OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get( omMetadataManager.getOzoneKey(volumeName, bucketName, key)); omKeyInfo.setKeyName(toKey); - OmRenameKeyInfo omRenameKeyInfo = new OmRenameKeyInfo(key, omKeyInfo); - renameKeyInfoList.add(omRenameKeyInfo); + formAndToKeyInfo.put(key, omKeyInfo); } + omRenameKeys = + new OmRenameKeys(volumeName, bucketName, null, formAndToKeyInfo); } } From 2dd7aee268f32ba109487135d8091693e43bb6a9 Mon Sep 17 00:00:00 2001 From: micah zhao <1020358092@qq.com> Date: Thu, 30 Jul 2020 22:15:08 +0800 Subject: [PATCH 13/15] Update hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java avoid use toString. Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com> --- .../apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index fa7411fba987..ef87101ad6a5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -242,7 +242,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, switch (result) { case SUCCESS: LOG.debug("Rename Keys is successfully completed for auditMap:{}.", - auditMap.toString()); + auditMap); break; case FAILURE: ozoneManager.getMetrics().incNumKeyRenameFails(); From ab6714b4fcc8a2bb238a6bf241d211fce5dad3a5 Mon Sep 17 00:00:00 2001 From: micah zhao <1020358092@qq.com> Date: Thu, 30 Jul 2020 22:15:49 +0800 Subject: [PATCH 14/15] Update hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java avoid use toString. Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com> --- .../apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index ef87101ad6a5..09177d5a8992 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -246,7 +246,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, break; case FAILURE: ozoneManager.getMetrics().incNumKeyRenameFails(); - LOG.error("Rename keys failed for auditMap:{}.", auditMap.toString()); + LOG.error("Rename keys failed for auditMap:{}.", auditMap); break; default: LOG.error("Unrecognized Result for OMKeysRenameRequest: {}", From 2bbba904762e77db377fc270722618c21b311bdb Mon Sep 17 00:00:00 2001 From: captainzmc Date: Fri, 31 Jul 2020 13:15:05 +0800 Subject: [PATCH 15/15] fix acquiredLock issue. --- .../ozone/om/request/key/OMKeysRenameRequest.java | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index 09177d5a8992..dbcde6d4ce14 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -113,6 +113,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, bucket.audit(auditMap); volumeName = bucket.realVolume(); bucketName = bucket.realBucket(); + acquiredLock = + omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, + volumeName, bucketName); for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) { @@ -183,9 +186,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, //Set modification time fromKeyValue.setModificationTime(Time.now()); - acquiredLock = - omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, - volumeName, bucketName); // Add to cache. // fromKey should be deleted, toKey should be added with newly updated // omKeyInfo. @@ -196,16 +196,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, new CacheValue<>(Optional.of(fromKeyValue), trxnLogIndex)); renamedKeys.put(fromKeyName, toKeyName); fromKeyAndToKeyInfo.put(fromKeyName, fromKeyValue); - if (acquiredLock) { - omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName, - bucketName); - acquiredLock = false; - } } - acquiredLock = - omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, - volumeName, bucketName); OmRenameKeys newOmRenameKeys = new OmRenameKeys(volumeName, bucketName, null, fromKeyAndToKeyInfo); omClientResponse = new OMKeysRenameResponse(omResponse