From ee1835f7537ce37864f255c61e721d9b90d54b6f Mon Sep 17 00:00:00 2001 From: KUAN-HAO HUANG <101171023+rich7420@users.noreply.github.com> Date: Sat, 3 Oct 2026 21:33:29 +0800 Subject: [PATCH 1/3] HDDS-16709. Remove redundant ListStatusLight RPC from S3 root listings in FSO buckets --- .../hadoop/ozone/client/OzoneBucket.java | 10 ++ .../hadoop/ozone/client/TestOzoneBucket.java | 133 ++++++++++++++++++ .../hadoop/ozone/om/TestListKeysWithFSO.java | 25 ++++ 3 files changed, 168 insertions(+) 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 29fa6de0ff6f..66c0fd2f8163 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 @@ -1884,10 +1884,20 @@ List getNextShallowListOfKeys(String prevKey) if (!addedKeyPrefix()) { initDelimiterKeyPrefix(); + final boolean listRoot = getKeyPrefix().isEmpty() && StringUtils.isEmpty(prevKey); if (!prepareStack(prevKey)) { return new ArrayList<>(); } + if (listRoot) { + startKey = ""; + findFirstStartKey = true; + setAddedKeyPrefix(true); + } + } + + // Root listings above are ready; other initial listings still need a start key. + if (!addedKeyPrefix()) { // 1. Get first element as startKey. List firstKeyResult = new ArrayList<>(); if (stack.isEmpty()) { diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneBucket.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneBucket.java index fd2c52faa638..ebed0276a5a5 100644 --- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneBucket.java +++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneBucket.java @@ -17,17 +17,44 @@ package org.apache.hadoop.ozone.client; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CLIENT_LIST_CACHE_SIZE; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; +import java.util.stream.IntStream; +import java.util.stream.Stream; +import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; import org.apache.hadoop.ozone.client.protocol.ClientProtocol; +import org.apache.hadoop.ozone.client.protocol.ListStatusLightOptions; +import org.apache.hadoop.ozone.om.helpers.BasicOmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; +import org.apache.hadoop.ozone.om.helpers.OzoneFileStatusLight; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; /** * Unit tests for {@link OzoneBucket}. @@ -69,4 +96,110 @@ public void clientProtocol3argDefaultDelegates() throws IOException { assertSame(status, proxy.getOzoneFileStatus("vol", "bucket", "key")); verify(proxy).getOzoneFileStatus("vol", "bucket", "key", false); } + + @ParameterizedTest + @NullAndEmptySource + void shallowRootListingStartsAtRoot(String prevKey) throws IOException { + ClientProtocol proxy = mock(ClientProtocol.class); + when(proxy.listStatusLight(any())).thenAnswer(invocation -> new ArrayList<>(Arrays.asList( + keyStatus("a-file", false), keyStatus("b-dir", true), keyStatus("c-file", false)))); + + Iterator keys = fsoBucket(proxy).listKeys("", prevKey, true); + + ArgumentCaptor options = ArgumentCaptor.forClass(ListStatusLightOptions.class); + verify(proxy).listStatusLight(options.capture()); + assertEquals("vol", options.getValue().getVolumeName()); + assertEquals("bucket", options.getValue().getBucketName()); + assertEquals("", options.getValue().getKeyName()); + assertEquals("", options.getValue().getStartKey()); + assertEquals("", options.getValue().getListPrefix()); + assertEquals(3, options.getValue().getNumEntries()); + assertFalse(options.getValue().isRecursive()); + assertFalse(options.getValue().isAllowPartialPrefixes()); + verifyNoMoreInteractions(proxy); + + OzoneKey file = keys.next(); + assertEquals("a-file", file.getName()); + assertEquals("owner", file.getOwner()); + assertEquals(10, file.getDataSize()); + assertEquals(RatisReplicationConfig.getInstance(ReplicationFactor.ONE), file.getReplicationConfig()); + OzoneKey directory = keys.next(); + assertEquals("b-dir/", directory.getName()); + assertFalse(directory.isFile()); + } + + @ParameterizedTest + @MethodSource("rootListings") + void shallowRootListingPreservesPages(String shape, int size, String prevKey) throws IOException { + ClientProtocol proxy = mock(ClientProtocol.class); + List statuses = new ArrayList<>(); + List expected = new ArrayList<>(); + for (int i = 0; i < size; i++) { + String name = "key-" + i; + boolean directory = shape.equals("directories") || (shape.equals("mixed") && i % 2 == 0); + statuses.add(keyStatus(name, directory)); + expected.add(name + (directory ? "/" : "")); + } + when(proxy.listStatusLight(any())).thenAnswer(invocation -> { + ListStatusLightOptions options = invocation.getArgument(0); + String startKey = OzoneFSUtils.removeTrailingSlashIfNeeded(options.getStartKey()); + int start = 0; + while (start < statuses.size() && statuses.get(start).getTrimmedName().compareTo(startKey) < 0) { + start++; + } + int end = Math.min(statuses.size(), start + (int) options.getNumEntries()); + return new ArrayList<>(statuses.subList(start, end)); + }); + + Iterator keys = fsoBucket(proxy).listKeys("", prevKey, true); + verify(proxy).listStatusLight(any()); + verifyNoMoreInteractions(proxy); + List actual = new ArrayList<>(); + keys.forEachRemaining(key -> actual.add(key.getName())); + assertEquals(expected, actual); + } + + private static Stream rootListings() { + return Stream.of("files", "directories", "mixed").flatMap(shape -> + IntStream.of(0, 1, 3, 4, 6, 7).boxed().flatMap(size -> + Stream.of(Arguments.of(shape, size, null), Arguments.of(shape, size, "")))); + } + + @ParameterizedTest + @ValueSource(strings = {"dir/", "/"}) + void shallowListingWithPrefixKeepsSeed(String prefix) throws IOException { + ClientProtocol proxy = mock(ClientProtocol.class); + when(proxy.listStatusLight(any())).thenAnswer(invocation -> + new ArrayList<>(Arrays.asList(keyStatus(prefix + "file", false)))); + + fsoBucket(proxy).listKeys(prefix, null, true); + + ArgumentCaptor options = ArgumentCaptor.forClass(ListStatusLightOptions.class); + verify(proxy, times(2)).listStatusLight(options.capture()); + assertTrue(options.getAllValues().get(0).isAllowPartialPrefixes()); + assertFalse(options.getAllValues().get(1).isAllowPartialPrefixes()); + } + + private static OzoneBucket fsoBucket(ClientProtocol proxy) { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.setInt(OZONE_CLIENT_LIST_CACHE_SIZE, 3); + return OzoneBucket.newBuilder(conf, proxy) + .setVolumeName("vol") + .setName("bucket") + .setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED) + .build(); + } + + private static OzoneFileStatusLight keyStatus(String name, boolean directory) { + BasicOmKeyInfo keyInfo = new BasicOmKeyInfo.Builder() + .setVolumeName("vol") + .setBucketName("bucket") + .setKeyName(name) + .setDataSize(10) + .setOwnerName("owner") + .setReplicationConfig(RatisReplicationConfig.getInstance(ReplicationFactor.ONE)) + .setIsFile(!directory) + .build(); + return new OzoneFileStatusLight(keyInfo, 0, directory); + } } 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 6929dac516ab..df57f3339d6c 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 @@ -27,6 +27,8 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.LinkedList; import java.util.List; @@ -505,6 +507,29 @@ public void testShallowListKeys() throws Exception { checkKeyShallowList(keyPrefix, startKey, expectedKeys, fsoOzoneBucket); } + @Test + void testShallowListKeysAtBucketRoot() throws Exception { + OzoneBucket bucket = DataTestUtil.createVolumeAndBucket(client, BucketLayout.FILE_SYSTEM_OPTIMIZED); + checkKeyShallowList("", null, Collections.emptyList(), bucket); + bucket.createDirectory("a"); + checkKeyShallowList("", null, Collections.singletonList("a/"), bucket); + + List files = Arrays.asList("b0", "b1", "b2", "b3", "b4"); + createAndAssertKeys(bucket, files); + List expected = new ArrayList<>(); + expected.add("a/"); + expected.addAll(files); + checkKeyShallowList("", null, expected, bucket); + checkKeyShallowList("", "", expected, bucket); + + createAndAssertKeys(bucket, Collections.singletonList("b5")); + expected.add("b5"); + checkKeyShallowList("", null, expected, bucket); + checkKeyShallowList("", "", expected, bucket); + checkKeyShallowList("", "b2", Arrays.asList("b3", "b4", "b5"), bucket); + checkKeyShallowList("b", null, Arrays.asList("b0", "b1", "b2", "b3", "b4", "b5"), bucket); + } + @Test void testIsFileFalseForDir() throws Exception { byte[] data = "key-data".getBytes(StandardCharsets.UTF_8); From 5cf7609e049e71d6d36c1843a3b82449f3dbef3f Mon Sep 17 00:00:00 2001 From: KUAN-HAO HUANG <101171023+rich7420@users.noreply.github.com> Date: Wed, 7 Oct 2026 16:05:04 +0800 Subject: [PATCH 2/3] HDDS-16709. Combine shallow listing initialization branches --- .../hadoop/ozone/client/OzoneBucket.java | 95 +++++++++---------- 1 file changed, 46 insertions(+), 49 deletions(-) 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 66c0fd2f8163..6a169ca9ef2d 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 @@ -1893,61 +1893,58 @@ List getNextShallowListOfKeys(String prevKey) startKey = ""; findFirstStartKey = true; setAddedKeyPrefix(true); - } - } - - // Root listings above are ready; other initial listings still need a start key. - if (!addedKeyPrefix()) { - // 1. Get first element as startKey. - List firstKeyResult = new ArrayList<>(); - if (stack.isEmpty()) { - // Case: startKey is empty - getChildrenKeys(getKeyPrefix(), prevKey, firstKeyResult); } else { - // Case: startKey is non-empty - while (!stack.isEmpty()) { - Pair keyPrefixPath = stack.pop(); - getChildrenKeys(keyPrefixPath.getLeft(), keyPrefixPath.getRight(), - firstKeyResult); - if (!firstKeyResult.isEmpty()) { - break; + // 1. Get first element as startKey. + List firstKeyResult = new ArrayList<>(); + if (stack.isEmpty()) { + // Case: startKey is empty + getChildrenKeys(getKeyPrefix(), prevKey, firstKeyResult); + } else { + // Case: startKey is non-empty + while (!stack.isEmpty()) { + Pair keyPrefixPath = stack.pop(); + getChildrenKeys(keyPrefixPath.getLeft(), keyPrefixPath.getRight(), + firstKeyResult); + if (!firstKeyResult.isEmpty()) { + break; + } } } - } - if (!firstKeyResult.isEmpty()) { - startKey = firstKeyResult.get(0).getName(); - findFirstStartKey = true; - } + if (!firstKeyResult.isEmpty()) { + startKey = firstKeyResult.get(0).getName(); + findFirstStartKey = true; + } - // A specific case where findFirstStartKey is false does not mean that - // the final result is empty because also need to determine whether - // keyPrefix is an existing key. Consider the following structure: - // te/ - // test1/ - // test1/file1 - // test1/file2 - // test2/ - // when keyPrefix='te' and prevKey='test1/file2', findFirstStartKey - // will be false because 'test1/file2' is the last key in dir - // 'test1/'. In the correct result for this case, 'test2/' is expected - // in the results and "test1/" should be excluded. - // - if (!findFirstStartKey) { - if (StringUtils.isBlank(prevKey) || !keyPrefixExist() - || !StringUtils.startsWith(prevKey, getKeyPrefix())) { - return new ArrayList<>(); + // A specific case where findFirstStartKey is false does not mean that + // the final result is empty because also need to determine whether + // keyPrefix is an existing key. Consider the following structure: + // te/ + // test1/ + // test1/file1 + // test1/file2 + // test2/ + // when keyPrefix='te' and prevKey='test1/file2', findFirstStartKey + // will be false because 'test1/file2' is the last key in dir + // 'test1/'. In the correct result for this case, 'test2/' is expected + // in the results and "test1/" should be excluded. + // + if (!findFirstStartKey) { + if (StringUtils.isBlank(prevKey) || !keyPrefixExist() + || !StringUtils.startsWith(prevKey, getKeyPrefix())) { + return new ArrayList<>(); + } } + // A special case where keyPrefix element should present in the + // resultList. See the annotation of #addKeyPrefixInfoToResultList + if (getKeyPrefix().equals(startKey) && findFirstStartKey + && !firstKeyResult.get(0).isFile()) { + resultList.add(firstKeyResult.get(0)); + } + // Note that the startKey needs to be an immediate child of the + // keyPrefix or black before calling listStatus. + startKey = adjustStartKey(startKey); + startKey = startKey == null ? "" : startKey; } - // A special case where keyPrefix element should present in the - // resultList. See the annotation of #addKeyPrefixInfoToResultList - if (getKeyPrefix().equals(startKey) && findFirstStartKey - && !firstKeyResult.get(0).isFile()) { - resultList.add(firstKeyResult.get(0)); - } - // Note that the startKey needs to be an immediate child of the - // keyPrefix or black before calling listStatus. - startKey = adjustStartKey(startKey); - startKey = startKey == null ? "" : startKey; } // 2. Get immediate children by listStatus method. From 9dc94a3980996f19afef62973fe06c48f2465bbf Mon Sep 17 00:00:00 2001 From: KUAN-HAO HUANG <101171023+rich7420@users.noreply.github.com> Date: Thu, 8 Oct 2026 23:16:12 +0800 Subject: [PATCH 3/3] HDDS-16709. Cover shallow root listing with a marker --- .../hadoop/ozone/client/TestOzoneBucket.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneBucket.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneBucket.java index ebed0276a5a5..1e98c05e131d 100644 --- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneBucket.java +++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneBucket.java @@ -165,6 +165,37 @@ private static Stream rootListings() { Stream.of(Arguments.of(shape, size, null), Arguments.of(shape, size, "")))); } + @Test + void shallowRootListingWithMarkerKeepsSeed() throws IOException { + ClientProtocol proxy = mock(ClientProtocol.class); + when(proxy.listStatusLight(any())).thenAnswer(invocation -> { + ListStatusLightOptions options = invocation.getArgument(0); + List statuses = new ArrayList<>(Arrays.asList( + keyStatus("a-file", false), keyStatus("b-file", false))); + statuses.removeIf(status -> status.getTrimmedName().compareTo(options.getStartKey()) < 0); + return statuses; + }); + + Iterator keys = fsoBucket(proxy).listKeys("", "a-file", true); + + ArgumentCaptor options = ArgumentCaptor.forClass(ListStatusLightOptions.class); + verify(proxy, times(2)).listStatusLight(options.capture()); + for (ListStatusLightOptions call : options.getAllValues()) { + assertEquals("vol", call.getVolumeName()); + assertEquals("bucket", call.getBucketName()); + assertEquals("", call.getKeyName()); + assertEquals("", call.getListPrefix()); + assertEquals(3, call.getNumEntries()); + assertFalse(call.isRecursive()); + } + assertEquals("a-file", options.getAllValues().get(0).getStartKey()); + assertTrue(options.getAllValues().get(0).isAllowPartialPrefixes()); + assertEquals("b-file", options.getAllValues().get(1).getStartKey()); + assertFalse(options.getAllValues().get(1).isAllowPartialPrefixes()); + verifyNoMoreInteractions(proxy); + assertEquals("b-file", keys.next().getName()); + } + @ParameterizedTest @ValueSource(strings = {"dir/", "/"}) void shallowListingWithPrefixKeepsSeed(String prefix) throws IOException {