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 58641027585..29f0aca9dc8 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 @@ -254,7 +254,6 @@ public final class OzoneManagerProtocolClientSideTranslatorPB private OmTransport transport; private ThreadLocal threadLocalS3Auth = new ThreadLocal<>(); - private boolean s3AuthCheck; public static final int BLOCK_ALLOCATION_RETRY_COUNT = 5; @@ -1033,7 +1032,7 @@ public ListKeysLightResult listKeysLight(String volumeName, reqBuilder.setBucketName(bucketName); reqBuilder.setCount(maxKeys); - if (StringUtils.isNotEmpty(startKey)) { + if (startKey != null) { reqBuilder.setStartKey(startKey); } @@ -2257,16 +2256,9 @@ public List listStatus(OmKeyArgs args, boolean recursive, .setSortDatanodes(args.getSortDatanodes()) .setLatestVersionLocation(args.getLatestVersionLocation()) .build(); - ListStatusRequest.Builder listStatusRequestBuilder = - ListStatusRequest.newBuilder() - .setKeyArgs(keyArgs) - .setRecursive(recursive) - .setStartKey(startKey) - .setNumEntries(numEntries); - if (allowPartialPrefixes) { - listStatusRequestBuilder.setAllowPartialPrefix(allowPartialPrefixes); - } + ListStatusRequest.Builder listStatusRequestBuilder = createListStatusRequestBuilder(keyArgs, recursive, startKey, + numEntries, allowPartialPrefixes); OMRequest omRequest = createOMRequest(Type.ListStatus) .setListStatusRequest(listStatusRequestBuilder.build()) @@ -2293,16 +2285,9 @@ public List listStatusLight(OmKeyArgs args, .setSortDatanodes(false) .setLatestVersionLocation(true) .build(); - ListStatusRequest.Builder listStatusRequestBuilder = - ListStatusRequest.newBuilder() - .setKeyArgs(keyArgs) - .setRecursive(recursive) - .setStartKey(startKey) - .setNumEntries(numEntries); - if (allowPartialPrefixes) { - listStatusRequestBuilder.setAllowPartialPrefix(allowPartialPrefixes); - } + ListStatusRequest.Builder listStatusRequestBuilder = createListStatusRequestBuilder(keyArgs, recursive, startKey, + numEntries, allowPartialPrefixes); OMRequest omRequest = createOMRequest(Type.ListStatusLight) .setListStatusRequest(listStatusRequestBuilder.build()) @@ -2319,6 +2304,26 @@ public List listStatusLight(OmKeyArgs args, return statusList; } + private ListStatusRequest.Builder createListStatusRequestBuilder(KeyArgs keyArgs, boolean recursive, String startKey, + long numEntries, boolean allowPartialPrefixes) { + ListStatusRequest.Builder listStatusRequestBuilder = + ListStatusRequest.newBuilder() + .setKeyArgs(keyArgs) + .setRecursive(recursive) + .setNumEntries(numEntries); + + if (startKey != null) { + listStatusRequestBuilder.setStartKey(startKey); + } else { + listStatusRequestBuilder.setStartKey(""); + } + + if (allowPartialPrefixes) { + listStatusRequestBuilder.setAllowPartialPrefix(allowPartialPrefixes); + } + return listStatusRequestBuilder; + } + @Override public List listStatus(OmKeyArgs args, boolean recursive, String startKey, long numEntries) throws IOException { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java index f499e3569c8..11594f3ef11 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java @@ -63,6 +63,8 @@ public class TestListKeysWithFSO { private static OzoneBucket fsoOzoneBucket; private static OzoneBucket legacyOzoneBucket2; private static OzoneBucket fsoOzoneBucket2; + private static OzoneBucket emptyLegacyOzoneBucket; + private static OzoneBucket emptyFsoOzoneBucket; private static OzoneClient client; /** @@ -105,6 +107,10 @@ public static void init() throws Exception { ozoneVolume.createBucket(fsoBucketName, omBucketArgs); fsoOzoneBucket2 = ozoneVolume.getBucket(fsoBucketName); + fsoBucketName = "bucket" + RandomStringUtils.randomNumeric(5); + ozoneVolume.createBucket(fsoBucketName, omBucketArgs); + emptyFsoOzoneBucket = ozoneVolume.getBucket(fsoBucketName); + builder = BucketArgs.newBuilder(); builder.setStorageType(StorageType.DISK); builder.setBucketLayout(BucketLayout.LEGACY); @@ -113,6 +119,10 @@ public static void init() throws Exception { ozoneVolume.createBucket(legacyBucketName, omBucketArgs); legacyOzoneBucket2 = ozoneVolume.getBucket(legacyBucketName); + legacyBucketName = "bucket" + RandomStringUtils.randomNumeric(5); + ozoneVolume.createBucket(legacyBucketName, omBucketArgs); + emptyLegacyOzoneBucket = ozoneVolume.getBucket(legacyBucketName); + initFSNameSpace(); } @@ -479,6 +489,23 @@ public void testShallowListKeys() throws Exception { expectedKeys = getExpectedKeyShallowList(keyPrefix, startKey, legacyOzoneBucket); checkKeyShallowList(keyPrefix, startKey, expectedKeys, fsoOzoneBucket); + + // case-7: keyPrefix corresponds to multiple existing keys and + // startKey is null in empty bucket + keyPrefix = "a1/b1/c12"; + startKey = null; + // a1/b1/c1222.tx + expectedKeys = + getExpectedKeyShallowList(keyPrefix, startKey, emptyLegacyOzoneBucket); + checkKeyShallowList(keyPrefix, startKey, expectedKeys, emptyFsoOzoneBucket); + + // case-8: keyPrefix corresponds to multiple existing keys and + // startKey is null + keyPrefix = "a1/b1/c12"; + // a1/b1/c1222.tx + expectedKeys = + getExpectedKeyShallowList(keyPrefix, startKey, legacyOzoneBucket); + checkKeyShallowList(keyPrefix, startKey, expectedKeys, fsoOzoneBucket); } /** diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListStatus.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListStatus.java index 52cb9287cc0..20977f9d483 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListStatus.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListStatus.java @@ -16,10 +16,10 @@ */ package org.apache.hadoop.ozone.om; -import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.ozone.TestDataUtil; import org.apache.hadoop.ozone.client.OzoneBucket; @@ -29,24 +29,30 @@ import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.List; +import java.util.stream.Stream; + +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.apache.hadoop.ozone.OzoneConfigKeys. - OZONE_FS_ITERATE_BATCH_SIZE; +import static org.junit.jupiter.params.provider.Arguments.arguments; /** * A simple test that asserts that list status output is sorted. */ @Timeout(1200) public class TestListStatus { + private static final Logger LOG = LoggerFactory.getLogger(TestListStatus.class); private static MiniOzoneCluster cluster = null; - private static OzoneConfiguration conf; private static OzoneBucket fsoOzoneBucket; private static OzoneClient client; @@ -54,11 +60,11 @@ public class TestListStatus { * Create a MiniDFSCluster for testing. *

* - * @throws IOException + * @throws IOException in case of I/O error */ @BeforeAll public static void init() throws Exception { - conf = new OzoneConfiguration(); + OzoneConfiguration conf = new OzoneConfiguration(); conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS, true); cluster = MiniOzoneCluster.newBuilder(conf).build(); @@ -69,7 +75,7 @@ public static void init() throws Exception { fsoOzoneBucket = TestDataUtil .createVolumeAndBucket(client, BucketLayout.FILE_SYSTEM_OPTIMIZED); - // Set the number of keys to be processed during batch operate. + // Set the number of keys to be processed during batch operated. conf.setInt(OZONE_FS_ITERATE_BATCH_SIZE, 5); buildNameSpaceTree(fsoOzoneBucket); @@ -83,44 +89,30 @@ public static void teardownClass() { } } - @Test - public void testSortedListStatus() throws Exception { - // a) test if output is sorted - checkKeyList("", "", 1000, 10, false); - - // b) number of keys returns is expected - checkKeyList("", "", 2, 2, false); - - // c) check if full prefix works - checkKeyList("a1", "", 100, 3, false); - - // d) check if full prefix with numEntries work - checkKeyList("a1", "", 2, 2, false); - - // e) check if existing start key >>> - checkKeyList("a1", "a1/a12", 100, 2, false); - - // f) check with non-existing start key - checkKeyList("", "a7", 100, 6, false); - - // g) check if half prefix works - checkKeyList("b", "", 100, 4, true); - - // h) check half prefix with non-existing start key - checkKeyList("b", "b5", 100, 2, true); - - // i) check half prefix with non-existing parent in start key - checkKeyList("b", "c", 100, 0, true); - - // i) check half prefix with non-existing parent in start key - checkKeyList("b", "b/g5", 100, 4, true); - - // i) check half prefix with non-existing parent in start key - checkKeyList("b", "c/g5", 100, 0, true); + @MethodSource("sortedListStatusParametersSource") + @ParameterizedTest(name = "{index} {5}") + public void testSortedListStatus(String keyPrefix, String startKey, int numEntries, int expectedNumKeys, + boolean isPartialPrefix, String testName) throws Exception { + checkKeyList(keyPrefix, startKey, numEntries, expectedNumKeys, isPartialPrefix); + } - // j) check prefix with non-existing prefix key - // and non-existing parent in start key - checkKeyList("a1/a111", "a1/a111/a100", 100, 0, true); + private static Stream sortedListStatusParametersSource() { + return Stream.of( + arguments("", "", 1000, 10, false, "Test if output is sorted"), + arguments("", "", 2, 2, false, "Number of keys returns is expected"), + arguments("a1", "", 100, 3, false, "Check if the full prefix works"), + arguments("a1", "", 2, 2, false, "Check if full prefix with numEntries work"), + arguments("a1", "a1/a12", 100, 2, false, "Check if existing start key >>>"), + arguments("", "a7", 100, 6, false, "Check with a non-existing start key"), + arguments("b", "", 100, 4, true, "Check if half-prefix works"), + arguments("b", "b5", 100, 2, true, "Check half prefix with non-existing start key"), + arguments("b", "c", 100, 0, true, "Check half prefix with non-existing parent in a start key"), + arguments("b", "b/g5", 100, 4, true, "Check half prefix with non-existing parent in a start key"), + arguments("b", "c/g5", 100, 0, true, "Check half prefix with non-existing parent in a start key"), + arguments("a1/a111", "a1/a111/a100", 100, 0, true, "Check prefix with a non-existing prefix key\n" + + " and non-existing parent in a start key"), + arguments("a1/a111", null, 100, 0, true, "Check start key is null") + ); } private static void createFile(OzoneBucket bucket, String keyName) @@ -131,6 +123,7 @@ private static void createFile(OzoneBucket bucket, String keyName) oos.flush(); } } + private static void buildNameSpaceTree(OzoneBucket ozoneBucket) throws Exception { /* @@ -172,33 +165,29 @@ private static void buildNameSpaceTree(OzoneBucket ozoneBucket) createFile(ozoneBucket, "/b8"); } - private void checkKeyList(String keyPrefix, String startKey, - long numEntries, int expectedNumKeys, - boolean isPartialPrefix) - throws Exception { + private void checkKeyList(String keyPrefix, String startKey, long numEntries, int expectedNumKeys, + boolean isPartialPrefix) throws Exception { List statuses = fsoOzoneBucket.listStatus(keyPrefix, false, startKey, numEntries, isPartialPrefix); assertEquals(expectedNumKeys, statuses.size()); - System.out.println("BEGIN:::keyPrefix---> " + keyPrefix + ":::---> " + - startKey); + LOG.info("BEGIN:::keyPrefix---> {} :::---> {}", keyPrefix, startKey); for (int i = 0; i < statuses.size() - 1; i++) { OzoneFileStatus stCurr = statuses.get(i); OzoneFileStatus stNext = statuses.get(i + 1); - System.out.println("status:" + stCurr); + LOG.info("status: {}", stCurr); assertThat(stCurr.getPath().compareTo(stNext.getPath())).isLessThan(0); } if (!statuses.isEmpty()) { OzoneFileStatus stNext = statuses.get(statuses.size() - 1); - System.out.println("status:" + stNext); + LOG.info("status: {}", stNext); } - System.out.println("END:::keyPrefix---> " + keyPrefix + ":::---> " + - startKey); + LOG.info("END:::keyPrefix---> {}:::---> {}", keyPrefix, startKey); } }