Skip to content

HDDS-16673. Avoid redundant ListBuckets RPCs when S3 bucket listing reaches the end #11397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rich7420 wants to merge 2 commits into
base: master
Choose a base branch
from
+114 −5
Open

HDDS-16673. Avoid redundant ListBuckets RPCs when S3 bucket listing reaches the end #11397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7042c0b
HDDS-16673. Avoid redundant ListBuckets RPCs when S3 bucket listing r…
rich7420 Oct 2, 2026
7143ec2
HDDS-16673. Clarify bucket listing regression coverage
rich7420 Oct 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/RootEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ private Response listDirectoryBuckets()
String accountId = S3Owner.DEFAULT_S3OWNER_ID;
int count = 0;
String lastBucketName = null;
while (bucketIterator.hasNext() && count < maxDirectoryBuckets) {
while (count < maxDirectoryBuckets && bucketIterator.hasNext()) {
OzoneBucket bucket = bucketIterator.next();
if (!bucket.getBucketLayout().isFileSystemOptimized()) {
continue;
Expand All @@ -130,7 +130,7 @@ private Response listDirectoryBuckets()
count++;
}

if (lastBucketName != null && bucketIterator.hasNext()) {
if (count == maxDirectoryBuckets && lastBucketName != null && bucketIterator.hasNext()) {
response.setContinuationToken(
new ContinueToken(lastBucketName, null).encodeToString());
}
Expand Down Expand Up @@ -182,7 +182,7 @@ private Response listAllBuckets()

int count = 0;
String lastBucketName = null;
while (bucketIterator.hasNext() && count < maxBuckets) {
while (count < maxBuckets && bucketIterator.hasNext()) {
OzoneBucket next = bucketIterator.next();
BucketMetadata bucketMetadata = new BucketMetadata();
bucketMetadata.setName(next.getName());
Expand All @@ -192,7 +192,7 @@ private Response listAllBuckets()
count++;
}

if (paginated && lastBucketName != null && bucketIterator.hasNext()) {
if (paginated && count == maxBuckets && lastBucketName != null && bucketIterator.hasNext()) {
response.setContinuationToken(
new ContinueToken(lastBucketName, null).encodeToString());
}
Expand Down
111 changes: 110 additions & 1 deletion hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,42 @@

package org.apache.hadoop.ozone.s3.endpoint;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
import javax.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.OzoneConfigKeys;
import org.apache.hadoop.ozone.client.ObjectStore;
import org.apache.hadoop.ozone.client.OzoneBucket;
import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.client.OzoneClientStub;
import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.s3.exception.OS3Exception;
import org.apache.hadoop.ozone.s3.signature.SignatureInfo;
import org.apache.hadoop.ozone.s3.util.ContinueToken;
import org.apache.hadoop.ozone.s3.util.S3Consts.QueryParams;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

/**
* This class test HeadBucket functionality.
Expand Down Expand Up @@ -156,6 +180,91 @@ public void testListAllBucketsUnpaginatedReturnsAll() throws Exception {
assertNull(response.getContinuationToken());
}

@ParameterizedTest(name = "directory={0}, buckets={1}, limit={2}, previous={3}")
@CsvSource({
// directory, total buckets, response limit, previous index, expected names, continuation token, RPC calls
"false, 0, 2, -1, '', false, 1",
"false, 1, 2, -1, bucket-0, false, 2",
"false, 2, 2, -1, bucket-0;bucket-1, false, 2",
"false, 3, 2, -1, bucket-0;bucket-1, true, 2",
"false, 2, 1, -1, bucket-0, true, 1",
"false, 3, 5, -1, bucket-0;bucket-1;bucket-2, false, 3",
"false, 3, 2, 1, bucket-2, false, 2",
"false, 3, , -1, bucket-0;bucket-1;bucket-2, false, 3",
"true, 0, 2, -1, '', false, 1",
"true, 1, 2, -1, bucket-0, false, 2",
"true, 2, 2, -1, bucket-0, false, 2",
"true, 3, 2, -1, bucket-0;bucket-2, false, 3",
"true, 4, 2, -1, bucket-0;bucket-2, true, 2",
"true, 2, 1, -1, bucket-0, true, 1",
"true, 3, 5, -1, bucket-0;bucket-2, false, 3",
"true, 3, 0, -1, '', false, 1",
"true, 5, 2, 2, bucket-4, false, 2",
"true, 2, 2, 0, '', false, 2"
})
void testListBucketsRpcCount(boolean directory, int total, Integer limit, int previous,
String expectedNames, boolean hasToken, int expectedCalls) throws Exception {
OzoneConfiguration conf = new OzoneConfiguration();
conf.setInt(OzoneConfigKeys.OZONE_CLIENT_LIST_CACHE_SIZE, 2);
ClientProtocol proxy = mock(ClientProtocol.class);
List<OzoneBucket> buckets = new ArrayList<>();
for (int i = 0; i < total; i++) {
buckets.add(OzoneBucket.newBuilder(conf, proxy).setVolumeName(DEFAULT_VOLUME).setName("bucket-" + i)
.setBucketLayout(i % 2 == 0 ? BucketLayout.FILE_SYSTEM_OPTIMIZED : BucketLayout.OBJECT_STORE).build());
}
when(proxy.listBuckets(eq(DEFAULT_VOLUME), isNull(), nullable(String.class), eq(2), eq(false)))
.thenAnswer(invocation -> {
String marker = invocation.getArgument(2);
return buckets.stream()
.filter(bucket -> marker == null || bucket.getName().compareTo(marker) > 0)
.limit(2).collect(Collectors.toList());
});
OzoneVolume volume = OzoneVolume.newBuilder(conf, proxy).setName(DEFAULT_VOLUME).setOwner("root")
.setAcls(Collections.emptyList()).build();
ObjectStore store = mock(ObjectStore.class);
when(store.getS3Volume()).thenReturn(volume);
when(store.getClientProxy()).thenReturn(proxy);
OzoneClient client = mock(OzoneClient.class);
when(client.getObjectStore()).thenReturn(store);
when(client.getProxy()).thenReturn(proxy);
RootEndpoint endpoint = EndpointBuilder.newRootEndpointBuilder().setClient(client)
.setSignatureInfo(new SignatureInfo.Builder(SignatureInfo.Version.V4)
.setCredentialScope("20260101/us-west-2/" + (directory ? "s3express" : "s3") + "/aws4_request").build())
.build();
if (limit != null) {
endpoint.queryParamsForTest().setInt(
directory ? QueryParams.MAX_DIRECTORY_BUCKETS : QueryParams.MAX_BUCKETS, limit);
}
if (previous >= 0) {
endpoint.queryParamsForTest().set(QueryParams.CONTINUATION_TOKEN,
new ContinueToken("bucket-" + previous, null).encodeToString());
}
try (Response httpResponse = endpoint.get()) {
Object entity = httpResponse.getEntity();
List<String> names;
String token;
if (directory) {
ListDirectoryBucketsResponse response = (ListDirectoryBucketsResponse) entity;
names = response.getBuckets().stream().map(bucket -> bucket.getName()).collect(Collectors.toList());
token = response.getContinuationToken();
} else {
ListBucketResponse response = (ListBucketResponse) entity;
names = response.getBuckets().stream().map(bucket -> bucket.getName()).collect(Collectors.toList());
token = response.getContinuationToken();
assertThat(response.getOwner().getDisplayName()).isEqualTo("root");
}
assertThat(names).containsExactly(StringUtils.split(expectedNames, ';'));
if (hasToken) {
assertThat(token).isNotNull();
assertThat(ContinueToken.decodeFromString(token).getLastKey()).isEqualTo(names.get(names.size() - 1));
} else {
assertThat(token).isNull();
}
}
verify(proxy, times(expectedCalls))
.listBuckets(eq(DEFAULT_VOLUME), isNull(), nullable(String.class), eq(2), eq(false));
}

private ListBucketResponse listWithMaxBuckets(int maxBuckets) throws Exception {
rootEndpoint.queryParamsForTest().unset(QueryParams.CONTINUATION_TOKEN);
rootEndpoint.queryParamsForTest().setInt(QueryParams.MAX_BUCKETS, maxBuckets);
Expand Down
Loading