From 06b1571afbf426d7d380873325a8fb9be1ca3e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Elek?= Date: Sun, 11 Aug 2019 14:32:00 +0200 Subject: [PATCH 1/3] HDDS-1950. S3 MPU part-list call fails if there are no parts --- .../hadoop/ozone/om/KeyManagerImpl.java | 12 ++- .../hadoop/ozone/om/TestKeyManagerUnit.java | 93 +++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index b58095f934a58..646180a93414f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1298,8 +1298,16 @@ public OmMultipartUploadListParts listParts(String volumeName, multipartKeyInfo.getPartKeyInfoMap(); Iterator> partKeyInfoMapIterator = partKeyInfoMap.entrySet().iterator(); - HddsProtos.ReplicationType replicationType = - partKeyInfoMap.firstEntry().getValue().getPartKeyInfo().getType(); + + OmKeyInfo omKeyInfo = + metadataManager.getOpenKeyTable().get(multipartKey); + + if (omKeyInfo == null) { + throw new IllegalStateException( + "Open key is missing for multipart upload " + multipartKey); + } + + HddsProtos.ReplicationType replicationType = omKeyInfo.getType(); int count = 0; List omPartInfoList = new ArrayList<>(); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java new file mode 100644 index 0000000000000..d699beb742245 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -0,0 +1,93 @@ +package org.apache.hadoop.ozone.om; + +import java.io.IOException; +import java.util.ArrayList; + +import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.protocol.StorageType; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType; +import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; +import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyArgs; +import org.apache.hadoop.ozone.om.helpers.OmKeyArgs.Builder; +import org.apache.hadoop.ozone.om.helpers.OmMultipartInfo; +import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadListParts; +import org.apache.hadoop.ozone.security.OzoneBlockTokenSecretManager; +import org.apache.hadoop.test.GenericTestUtils; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +/** + * Unit test key manager. + */ +public class TestKeyManagerUnit { + + private OmMetadataManagerImpl metadataManager; + private KeyManagerImpl keyManager; + + @Before + public void setup() throws IOException { + OzoneConfiguration configuration = new OzoneConfiguration(); + configuration.set(HddsConfigKeys.OZONE_METADATA_DIRS, + GenericTestUtils.getRandomizedTestDir().toString()); + metadataManager = new OmMetadataManagerImpl(configuration); + keyManager = new KeyManagerImpl( + Mockito.mock(ScmBlockLocationProtocol.class), + metadataManager, + configuration, + "omtest", + Mockito.mock(OzoneBlockTokenSecretManager.class) + ); + } + + @Test + public void listMultipartUploadPartsWithZeroUpload() throws IOException { + //GIVEN + createBucket(metadataManager, "vol1", "bucket1"); + + OmMultipartInfo omMultipartInfo = + initMultipartUpload(keyManager, "vol1", "bucket1", "dir/key1"); + + //WHEN + OmMultipartUploadListParts omMultipartUploadListParts = keyManager + .listParts("vol1", "bucket1", "dir/key1", omMultipartInfo.getUploadID(), + 0, 10); + + Assert.assertEquals(0, + omMultipartUploadListParts.getPartInfoList().size()); + + } + + private void createBucket(OmMetadataManagerImpl omMetadataManager, + String volume, String bucket) + throws IOException { + omMetadataManager.getBucketTable() + .put(omMetadataManager.getBucketKey(volume, bucket), + OmBucketInfo.newBuilder() + .setVolumeName(volume) + .setBucketName(bucket) + .setStorageType(StorageType.DISK) + .setIsVersionEnabled(false) + .setAcls(new ArrayList<>()) + .build()); + } + + private OmMultipartInfo initMultipartUpload(KeyManagerImpl omtest, + String volume, String bucket, String key) + throws IOException { + OmKeyArgs key1 = new Builder() + .setVolumeName(volume) + .setBucketName(bucket) + .setKeyName(key) + .setType(ReplicationType.RATIS) + .setFactor(ReplicationFactor.THREE) + .setAcls(new ArrayList<>()) + .build(); + return omtest.initiateMultipartUpload(key1); + } +} \ No newline at end of file From 17ef16f4a92673ad86013245d13baff2dcebcb12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Elek?= Date: Wed, 21 Aug 2019 08:27:34 +0200 Subject: [PATCH 2/3] Open key table read should be checked only if there are no parts. --- .../hadoop/ozone/om/KeyManagerImpl.java | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 646180a93414f..52b753a70d7ce 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1299,15 +1299,21 @@ public OmMultipartUploadListParts listParts(String volumeName, Iterator> partKeyInfoMapIterator = partKeyInfoMap.entrySet().iterator(); - OmKeyInfo omKeyInfo = - metadataManager.getOpenKeyTable().get(multipartKey); + HddsProtos.ReplicationType replicationType = null; - if (omKeyInfo == null) { - throw new IllegalStateException( - "Open key is missing for multipart upload " + multipartKey); + //if there are no parts, use the replicationType from the open key. + if (partKeyInfoMap.size() == 0) { + OmKeyInfo omKeyInfo = + metadataManager.getOpenKeyTable().get(multipartKey); + + if (omKeyInfo == null) { + throw new IllegalStateException( + "Open key is missing for multipart upload " + multipartKey); + } + + replicationType = omKeyInfo.getType(); } - HddsProtos.ReplicationType replicationType = omKeyInfo.getType(); int count = 0; List omPartInfoList = new ArrayList<>(); @@ -1324,11 +1330,16 @@ public OmMultipartUploadListParts listParts(String volumeName, partKeyInfo.getPartKeyInfo().getModificationTime(), partKeyInfo.getPartKeyInfo().getDataSize()); omPartInfoList.add(omPartInfo); + + //if there are parts, use replication type from one of the parts replicationType = partKeyInfo.getPartKeyInfo().getType(); count++; } } + Preconditions.checkNotNull(replicationType, + "Replication type can't be identified"); + if (partKeyInfoMapIterator.hasNext()) { Map.Entry partKeyInfoEntry = partKeyInfoMapIterator.next(); From f513004250cbaddd4815a2dbe17e6a4352801dfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Elek?= Date: Wed, 28 Aug 2019 13:29:42 +0200 Subject: [PATCH 3/3] fix rat and unit test issues --- .../hadoop/ozone/om/KeyManagerImpl.java | 27 ++++++++++--------- .../hadoop/ozone/om/TestKeyManagerUnit.java | 18 +++++++++++++ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 52b753a70d7ce..4f56160b9dabd 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1301,19 +1301,6 @@ public OmMultipartUploadListParts listParts(String volumeName, HddsProtos.ReplicationType replicationType = null; - //if there are no parts, use the replicationType from the open key. - if (partKeyInfoMap.size() == 0) { - OmKeyInfo omKeyInfo = - metadataManager.getOpenKeyTable().get(multipartKey); - - if (omKeyInfo == null) { - throw new IllegalStateException( - "Open key is missing for multipart upload " + multipartKey); - } - - replicationType = omKeyInfo.getType(); - } - int count = 0; List omPartInfoList = new ArrayList<>(); @@ -1337,6 +1324,20 @@ public OmMultipartUploadListParts listParts(String volumeName, } } + if (replicationType == null) { + //if there are no parts, use the replicationType from the open key. + + OmKeyInfo omKeyInfo = + metadataManager.getOpenKeyTable().get(multipartKey); + + if (omKeyInfo == null) { + throw new IllegalStateException( + "Open key is missing for multipart upload " + multipartKey); + } + + replicationType = omKeyInfo.getType(); + + } Preconditions.checkNotNull(replicationType, "Replication type can't be identified"); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index d699beb742245..a5a446c34031d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -1,3 +1,21 @@ +/* + * 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 java.io.IOException;