Skip to content
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.

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

Support maxKeys for list request #2779

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public class MySqlNamedBlobDbConfig {
* The maximum number of entries to return per response page when listing blobs.
*/
@Config(LIST_MAX_RESULTS)
@Default("100")
@Default("1000")
public final int listMaxResults;

/**
Expand Down
17 changes: 10 additions & 7 deletions ambry-api/src/main/java/com/github/ambry/named/NamedBlobDb.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,20 @@ default CompletableFuture<NamedBlobRecord> get(String accountName, String contai
/**
* List blobs that start with a provided prefix in a container. This returns paginated results. If there are
* additional pages to read, {@link Page#getNextPageToken()} will be non null.
* @param accountName the name of the account.
* @param containerName the name of the container.
*
* @param accountName the name of the account.
* @param containerName the name of the container.
* @param blobNamePrefix the name prefix to search for.
* @param pageToken if {@code null}, return the first page of {@link NamedBlobRecord}s that start with
* {@code blobNamePrefix}. If set, use this as a token to resume reading additional pages
* of records that start with the prefix.
* @param pageToken if {@code null}, return the first page of {@link NamedBlobRecord}s that start with
* {@code blobNamePrefix}. If set, use this as a token to resume reading additional pages of
* records that start with the prefix.
* @param maxKey the maximum number of keys returned in the response. By default, the action returns up to listMaxResults
* which can be tuned by config.
* @return a {@link CompletableFuture} that will eventually contain a {@link Page} of {@link NamedBlobRecord}s
* starting with the specified prefix or an exception if an error occurred.
* starting with the specified prefix or an exception if an error occurred.
*/
CompletableFuture<Page<NamedBlobRecord>> list(String accountName, String containerName, String blobNamePrefix,
String pageToken);
String pageToken, String maxKey);
SophieGuo410 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Persist a {@link NamedBlobRecord} in the database.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import org.slf4j.LoggerFactory;

import static com.github.ambry.frontend.FrontendUtils.*;
import static com.github.ambry.frontend.s3.S3ListHandler.*;
import static com.github.ambry.rest.RestUtils.*;


/**
Expand Down Expand Up @@ -129,9 +131,10 @@ private Callback<Void> securityProcessRequestCallback() {
private Callback<Void> securityPostProcessRequestCallback() {
return buildCallback(frontendMetrics.listSecurityPostProcessRequestMetrics, securityCheckResult -> {
NamedBlobPath namedBlobPath = NamedBlobPath.parse(RestUtils.getRequestPath(restRequest), restRequest.getArgs());
String maxKeys = getHeader(restRequest.getArgs(), MAXKEYS_PARAM_NAME, false);
CallbackUtils.callCallbackAfter(
namedBlobDb.list(namedBlobPath.getAccountName(), namedBlobPath.getContainerName(),
namedBlobPath.getBlobNamePrefix(), namedBlobPath.getPageToken()), listBlobsCallback());
namedBlobPath.getBlobNamePrefix(), namedBlobPath.getPageToken(), maxKeys), listBlobsCallback());
}, uri, LOGGER, finalCallback);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import com.github.ambry.config.QuotaConfig;
import com.github.ambry.config.RouterConfig;
import com.github.ambry.config.VerifiableProperties;
import com.github.ambry.frontend.s3.S3MultipartUploadHandler;
import com.github.ambry.messageformat.BlobInfo;
import com.github.ambry.messageformat.BlobProperties;
import com.github.ambry.named.DeleteResult;
Expand Down Expand Up @@ -2525,17 +2524,17 @@ private void doListNamedBlobsTest(String prefix, String pageToken, Page<NamedBlo
RestRequest restRequest = createRestRequest(RestMethod.GET, path, null, null);
MockRestResponseChannel restResponseChannel = new MockRestResponseChannel();
if (pageToReturn != null) {
when(namedBlobDb.list(any(), any(), any(), any())).thenReturn(CompletableFuture.completedFuture(pageToReturn));
when(namedBlobDb.list(any(), any(), any(), any(), any())).thenReturn(CompletableFuture.completedFuture(pageToReturn));
} else {
CompletableFuture<Page<NamedBlobRecord>> future = new CompletableFuture<>();
future.completeExceptionally(new RestServiceException("NamedBlobDb error", expectedErrorCode));
when(namedBlobDb.list(any(), any(), any(), any())).thenReturn(future);
when(namedBlobDb.list(any(), any(), any(), any(), any())).thenReturn(future);
}

if (expectedErrorCode == null) {
assertNotNull("pageToReturn should be set", pageToReturn);
doOperation(restRequest, restResponseChannel);
verify(namedBlobDb).list(refAccount.getName(), refContainer.getName(), prefix, pageToken);
verify(namedBlobDb).list(refAccount.getName(), refContainer.getName(), prefix, pageToken, null);
Page<NamedBlobListEntry> response =
Page.fromJson(new JSONObject(new String(restResponseChannel.getResponseBody())), NamedBlobListEntry::new);
assertEquals("Unexpected blobs returned",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public CompletableFuture<NamedBlobRecord> get(String accountName, String contain

@Override
public CompletableFuture<Page<NamedBlobRecord>> list(String accountName, String containerName, String blobNamePrefix,
String pageToken) {
String pageToken, String maxKey) {
if (exception != null) {
return FutureUtils.completedExceptionally(exception);
}
Expand Down Expand Up @@ -113,7 +113,8 @@ public CompletableFuture<Page<NamedBlobRecord>> list(String accountName, String

NamedBlobRecord record = recordList.get(recordList.size() - 1).getFirst();
long deleteTs = recordList.get(recordList.size() - 1).getSecond().getSecond();
if (numRecords++ == listMaxResults) {
int maxKeysValue = maxKey == null ? listMaxResults : Integer.parseInt(maxKey);
if (numRecords++ == maxKeysValue) {
nextContinuationToken = record.getBlobName();
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public void testPutGetListDeleteSequence() throws Exception {
time.setCurrentMilliseconds(System.currentTimeMillis());
for (Account account : accountService.getAllAccounts()) {
for (Container container : account.getAllContainers()) {
Page<NamedBlobRecord> page = namedBlobDb.list(account.getName(), container.getName(), "name", null).get();
Page<NamedBlobRecord> page = namedBlobDb.list(account.getName(), container.getName(), "name", null, null).get();
assertNull("No continuation token expected", page.getNextPageToken());
assertEquals("Unexpected number of blobs in container", blobsPerContainer, page.getEntries().size());
}
Expand All @@ -133,12 +133,19 @@ public void testPutGetListDeleteSequence() throws Exception {
for (Account account : accountService.getAllAccounts()) {
for (Container container : account.getAllContainers()) {
//page with no token
Page<NamedBlobRecord> page = namedBlobDb.list(account.getName(), container.getName(), null, null).get();
Page<NamedBlobRecord> page = namedBlobDb.list(account.getName(), container.getName(), null, null, null).get();
assertNull("No continuation token expected", page.getNextPageToken());
assertEquals("Unexpected number of blobs in container", blobsPerContainer, page.getEntries().size());
//page with token
Page<NamedBlobRecord> pageWithToken = namedBlobDb.list(account.getName(), container.getName(), null, "name/4").get();
assertEquals("Unexpected number of blobs in container", blobsPerContainer / 5, pageWithToken.getEntries().size());
Page<NamedBlobRecord> pageWithToken =
namedBlobDb.list(account.getName(), container.getName(), null, "name/4", null).get();
assertEquals("Unexpected number of blobs in container", blobsPerContainer / 5,
pageWithToken.getEntries().size());
//page with maxKeys
Page<NamedBlobRecord> pageWithMaxKey =
namedBlobDb.list(account.getName(), container.getName(), null, null, "1").get();
assertEquals("Unexpected number of blobs in container", blobsPerContainer / 5,
pageWithMaxKey.getEntries().size());
}
}

Expand Down Expand Up @@ -293,7 +300,7 @@ public void testListNamedBlobs() throws Exception {
);

// List named blob should only put out valid ones without empty entries.
Page<NamedBlobRecord> page = namedBlobDb.list(account.getName(), container.getName(), blobNamePrefix, null).get();
Page<NamedBlobRecord> page = namedBlobDb.list(account.getName(), container.getName(), blobNamePrefix, null, null).get();
assertEquals("List named blob entries should match the valid records", validRecords, new HashSet<>(page.getEntries()));
assertNull("Next page token should be null", page.getNextPageToken());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,12 @@ public CompletableFuture<NamedBlobRecord> get(String accountName, String contain

@Override
public CompletableFuture<Page<NamedBlobRecord>> list(String accountName, String containerName, String blobNamePrefix,
String pageToken) {
String pageToken, String maxKeys) {
return executeTransactionAsync(accountName, containerName, true, (accountId, containerId, connection) -> {
long startTime = this.time.milliseconds();
Page<NamedBlobRecord> recordPage =
run_list_v2(accountName, containerName, blobNamePrefix, pageToken, accountId, containerId, connection);
run_list_v2(accountName, containerName, blobNamePrefix, pageToken, accountId, containerId, connection,
maxKeys);
metricsRecoder.namedBlobListTimeInMs.update(this.time.milliseconds() - startTime);
return recordPage;
}, null);
Expand Down Expand Up @@ -623,9 +624,10 @@ private NamedBlobRecord run_get_v2(String accountName, String containerName, Str
}

private Page<NamedBlobRecord> run_list_v2(String accountName, String containerName, String blobNamePrefix,
String pageToken, short accountId, short containerId, Connection connection) throws Exception {
String pageToken, short accountId, short containerId, Connection connection, String maxKeys) throws Exception {
String query = "";
String queryStatement = blobNamePrefix == null? LIST_ALL_QUERY_V2: LIST_QUERY_V2;
String queryStatement = blobNamePrefix == null ? LIST_ALL_QUERY_V2 : LIST_QUERY_V2;
int maxKeysValue = maxKeys == null ? config.listMaxResults : Integer.parseInt(maxKeys);
try (PreparedStatement statement = connection.prepareStatement(queryStatement)) {
statement.setInt(1, accountId);
statement.setInt(2, containerId);
Expand All @@ -636,7 +638,7 @@ private Page<NamedBlobRecord> run_list_v2(String accountName, String containerNa
statement.setString(3, blobNamePrefix + "%");
statement.setString(4, pageToken != null ? pageToken : blobNamePrefix);
}
statement.setInt(5, config.listMaxResults + 1);
statement.setInt(5, maxKeysValue + 1);
query = statement.toString();
logger.debug("Getting list of blobs matching prefix {} from MySql. Query {}", blobNamePrefix, query);
try (ResultSet resultSet = statement.executeQuery()) {
Expand All @@ -645,7 +647,7 @@ private Page<NamedBlobRecord> run_list_v2(String accountName, String containerNa
int resultIndex = 0;
while (resultSet.next()) {
String blobName = resultSet.getString(1);
if (resultIndex++ == config.listMaxResults) {
if (resultIndex++ == maxKeysValue) {
nextContinuationToken = blobName;
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ public void run() {
int numberOfList = 0;
for (NamedBlobRecord record : allRecords) {
if (!record.getAccountName().equals(String.format(ACCOUNT_NAME_FORMAT, HUGE_LIST_ACCOUNT_ID))) {
namedBlobDb.list(record.getAccountName(), record.getContainerName(), "A", null).get();
namedBlobDb.list(record.getAccountName(), record.getContainerName(), "A", null, null).get();
numberOfList++;
if (numberOfList == 100) {
break;
Expand All @@ -543,7 +543,7 @@ public void run() {
String token = null;
for (int i = 0; i < 100; i++) {
token =
namedBlobDb.list(accountName, containerName, HUGE_LIST_COMMON_PREFIX, token).get().getNextPageToken();
namedBlobDb.list(accountName, containerName, HUGE_LIST_COMMON_PREFIX, token, null).get().getNextPageToken();
}
System.out.println("PerformanceTestWorker " + id + " finishes listing for huge records");
}
Expand Down