Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class MySqlNamedBlobDbConfig {
public static final String LIST_MAX_RESULTS = PREFIX + "list.max.results";
public static final String QUERY_STALE_DATA_MAX_RESULTS = PREFIX + "query.stale.data.max.results";
public static final String STALE_DATA_RETENTION_DAYS = PREFIX + "stale.data.retention.days";
public static final String LIST_QUERY_TIMEOUT_SECONDS = PREFIX + "list.query.timeout.seconds";
public static final String TRANSACTION_ISOLATION_LEVEL = PREFIX + "transaction.isolation.level";
public static final String LIST_NAMED_BLOBS_SQL_OPTION = "list.named.blobs.sql.option";
public static final String ENABLE_HARD_DELETE = PREFIX + "enable.hard.delete";
Expand Down Expand Up @@ -117,6 +118,20 @@ public class MySqlNamedBlobDbConfig {
@Default("5")
public final int staleDataRetentionDays;

/**
* Per-statement timeout (in seconds) applied to LIST queries via {@link java.sql.Statement#setQueryTimeout(int)}.
* Guards against a LIST scanning an unexpectedly large container: when the budget is exceeded the JDBC driver
* issues a clean cancel and the server throws a {@link java.sql.SQLException} (e.g. MySQLTimeoutException),
* which surfaces as an ordinary error rather than tearing the socket ("Communications link failure" -> HTTP 500).
*
* <p><b>Default 0 disables the timeout</b>, making this a no-op for existing deployments. Operators on fabrics
* where a network/socket timeout is shorter than the server statement kill should set this just below that
* socket timeout so a slow LIST fails fast and cleanly instead of poisoning the connection.
*/
@Config(LIST_QUERY_TIMEOUT_SECONDS)
@Default("0")
public final int listQueryTimeoutSeconds;

/**
* Transaction isolation level to be set on DB Connection. When nothing is set, default MySQL DB transaction level
* (REPEATABLE_READ) will take effect.
Expand Down Expand Up @@ -164,6 +179,8 @@ public MySqlNamedBlobDbConfig(VerifiableProperties verifiableProperties) {
verifiableProperties.getIntInRange(QUERY_STALE_DATA_MAX_RESULTS, 1000, 1, Integer.MAX_VALUE);
this.staleDataRetentionDays =
verifiableProperties.getIntInRange(STALE_DATA_RETENTION_DAYS, 5, 1, Integer.MAX_VALUE);
this.listQueryTimeoutSeconds =
verifiableProperties.getIntInRange(LIST_QUERY_TIMEOUT_SECONDS, 0, 0, Integer.MAX_VALUE);
this.transactionIsolationLevel =
verifiableProperties.getEnum(TRANSACTION_ISOLATION_LEVEL, TransactionIsolationLevel.class,
TransactionIsolationLevel.TRANSACTION_NONE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,15 +327,90 @@ private void doListBlob(String account, String container, String key) throws Exc
assertEquals("Mismatch in encoding type", "url", listBucketResult.getEncodingType());
}

/**
* Reproduces the request shape that the AWS S3 SDK emits when the caller sets an unset/empty
* prefix on ListObjectsRequest: the query string contains {@code prefix=} (parameter present,
* value empty) rather than omitting the parameter entirely. This is the exact shape that
* caused the original empty-prefix regression on option 4 (LIST_WITH_PREFIX_SQL evaluated
* {@code blob_name LIKE '%'} over a full container scan, timing out at MAX_EXECUTION_TIME);
* fixed in linkedin/ambry#3265 by collapsing empty {@code prefix} to {@code null} in
* {@link NamedBlobPath#parseS3}.
*
* This test asserts the request path returns 200 OK end-to-end through the S3 handler stack
* (HTTP → Netty → {@code S3ListHandler} → {@code NamedBlobPath.parseS3} → {@code
* NamedBlobListHandler} → {@code NamedBlobDb#list}). The named-blob DB in this integration
* test is {@code InMemNamedBlobDbFactory}, so this test specifically catches regressions in
* the S3-handler routing layer (empty-prefix collapse, parseS3 logic) — not SQL-side
* regressions, which are covered by
* {@code MySqlNamedBlobDbListOperationIntegrationTest#testListNamedBlobsWithNullPrefix}
* against a real MySQL backend.
*/
@Test
public void s3ListEmptyPrefixTest() throws Exception {
Container container = ACCOUNT.getAllContainers().iterator().next();
String account = ACCOUNT.getName();
String containerName = container.getName();

// Seed a couple of blobs so the LIST has something to return — the test focuses on the
// request shape and routing, not on the content of the response.
String[] keys = new String[]{"empty_prefix_seed_a", "empty_prefix_seed_b"};
int contentSize = 64;
for (String key : keys) {
byte[] content = TestUtils.getRandomBytes(contentSize);
doPutBlob(account, containerName, key, contentSize, content);
}

// V1 LIST: GET /s3/{account}/{container}?prefix= (explicit empty value)
String uriV1 = String.format("/s3/%s/%s?prefix=", account, containerName);
HttpHeaders headers = new DefaultHttpHeaders();
FullHttpRequest reqV1 = buildRequest(HttpMethod.GET, uriV1, headers, null);
NettyClient.ResponseParts partsV1 = nettyClient.sendRequest(reqV1, null, null).get();
HttpResponse respV1 = getHttpResponse(partsV1);
assertEquals("LIST v1 with explicit empty prefix should return 200 OK end-to-end through "
+ "the S3 handler stack; regression in parseS3 empty-prefix collapse would surface here",
HttpResponseStatus.OK, respV1.status());

// V2 LIST: GET /s3/{account}/{container}?prefix=&list-type=2
String uriV2 = String.format("/s3/%s/%s?prefix=&list-type=2", account, containerName);
FullHttpRequest reqV2 = buildRequest(HttpMethod.GET, uriV2, new DefaultHttpHeaders(), null);
NettyClient.ResponseParts partsV2 = nettyClient.sendRequest(reqV2, null, null).get();
HttpResponse respV2 = getHttpResponse(partsV2);
assertEquals("LIST v2 with explicit empty prefix should return 200 OK end-to-end",
HttpResponseStatus.OK, respV2.status());

// Cleanup
for (String key : keys) {
String deleteUri = String.format("/s3/%s/%s/%s", account, containerName, key);
FullHttpRequest delReq = buildRequest(HttpMethod.DELETE, deleteUri, new DefaultHttpHeaders(), null);
nettyClient.sendRequest(delReq, null, null).get();
}
}

/**
* Builds properties required to start a {@link RestServer} as an Ambry frontend server.
* @param trustStoreFile the trust store file to add certificates to for SSL testing.
* @param account {@link Account} for which quota needs to be specified.
* @return a {@link VerifiableProperties} with the parameters for an Ambry frontend server.
*/
private static VerifiableProperties buildFrontendVPropsForQuota(File trustStoreFile, Account account)
static VerifiableProperties buildFrontendVPropsForQuota(File trustStoreFile, Account account)
throws IOException, GeneralSecurityException {
Properties properties = buildFrontendVProps(trustStoreFile);
return buildFrontendVPropsForQuota(trustStoreFile, account, "com.github.ambry.commons.InMemNamedBlobDbFactory",
null);
}

/**
* Builds quota-enabled frontend properties, letting the caller pick the named-blob DB factory (e.g. the
* MySQL-backed factory) and supply extra properties (e.g. the dbInfo and LIST SQL option). Reused by
* sibling integration tests that exercise the S3 stack against a real backend.
* @param trustStoreFile the trust store file to add certificates to for SSL testing.
* @param account {@link Account} for which quota needs to be specified.
* @param namedBlobDbFactory the fully-qualified {@link com.github.ambry.named.NamedBlobDbFactory} class name.
* @param extraProps additional properties to layer on top (may be null).
* @return a {@link VerifiableProperties} with the parameters for an Ambry frontend server.
*/
static VerifiableProperties buildFrontendVPropsForQuota(File trustStoreFile, Account account,
String namedBlobDbFactory, Properties extraProps) throws IOException, GeneralSecurityException {
Properties properties = buildFrontendVProps(trustStoreFile, namedBlobDbFactory, extraProps);
JSONObject cuResourceQuotaJson = new JSONObject();
JSONObject quotaJson = new JSONObject();
quotaJson.put("rcu", 10737418240L);
Expand All @@ -354,7 +429,18 @@ private static VerifiableProperties buildFrontendVPropsForQuota(File trustStoreF
* @param trustStoreFile the trust store file to add certificates to for SSL testing.
* @return a {@link Properties} with the parameters for an Ambry frontend server.
*/
private static Properties buildFrontendVProps(File trustStoreFile)
static Properties buildFrontendVProps(File trustStoreFile) throws IOException, GeneralSecurityException {
return buildFrontendVProps(trustStoreFile, "com.github.ambry.commons.InMemNamedBlobDbFactory", null);
}

/**
* Builds frontend properties with a caller-selected named-blob DB factory and optional extra properties.
* @param trustStoreFile the trust store file to add certificates to for SSL testing.
* @param namedBlobDbFactory the fully-qualified {@link com.github.ambry.named.NamedBlobDbFactory} class name.
* @param extraProps additional properties to layer on top (may be null).
* @return a {@link Properties} with the parameters for an Ambry frontend server.
*/
static Properties buildFrontendVProps(File trustStoreFile, String namedBlobDbFactory, Properties extraProps)
throws IOException, GeneralSecurityException {
Properties properties = new Properties();
properties.put("rest.server.rest.request.service.factory",
Expand All @@ -377,8 +463,11 @@ private static Properties buildFrontendVProps(File trustStoreFile)
properties.setProperty("clustermap.datacenter.name", DATA_CENTER_NAME);
properties.setProperty("clustermap.host.name", HOST_NAME);
properties.setProperty(FrontendConfig.ENABLE_UNDELETE, Boolean.toString(true));
properties.setProperty(FrontendConfig.NAMED_BLOB_DB_FACTORY, "com.github.ambry.commons.InMemNamedBlobDbFactory");
properties.setProperty(FrontendConfig.NAMED_BLOB_DB_FACTORY, namedBlobDbFactory);
properties.setProperty(MySqlNamedBlobDbConfig.LIST_MAX_RESULTS, String.valueOf(NAMED_BLOB_LIST_RESULT_MAX));
if (extraProps != null) {
properties.putAll(extraProps);
}
return properties;
}
}
Loading
Loading