Skip to content

Commit 0e1c73c

Browse files
zichenglclaude
andcommitted
Fix option-4 LIST_ALL (empty-prefix S3 LIST) timeout/HTTP 500 on large containers
An empty-prefix S3 ListObjects collapses to a null prefix (#3265) and routes to LIST_ALL. Under listNamedBlobsSqlOption=4, LIST_ALL used a window-function query (MAX(version) OVER (PARTITION BY blob_name)) whose derived table is materialized in full before the outer LIMIT -- it scans the entire container and cannot early-terminate. On large containers it ran past a short socket timeout, threw "Communications link failure", and returned HTTP 500. Because the empty-prefix path is the only way an S3 client reaches LIST_ALL, this blocks the option-4 rollout: any LIST with an empty prefix on a large container would 500. Fix: - Replace the option-4 LIST_ALL window query with a correlated-subquery form (the LIST_WITH_PREFIX option-3 shape, minus the prefix predicate). The outer scans candidates in PRIMARY KEY (blob_name) order and keeps only version = MAX(version); deleted_ts sits on the outer candidate, so a soft-deleted latest version hides the blob entirely (no older-version leak) -- preserving option-4 semantics. With the PK already blob_name-ordered, ORDER BY blob_name LIMIT N early-terminates after N matches instead of materializing the whole container. Options 2/3 LIST_ALL are unchanged. - run_list_v2 now switches the null-prefix binder by option (2/3 -> 5 params, 4 -> new constructListAllQueryV4 -> 7 params) to match the statement. - Add a default-off list query-timeout knob (mysql.named.blob.list.query.timeout.seconds) applied via Statement.setQueryTimeout, so a residual slow LIST surfaces a clean SQLException instead of tearing the connection (500). No-op for existing fabrics. Testing Done: - ./gradlew :ambry-named-mysql:intTest --tests "*MySqlNamedBlobDbListOperationIntegrationTest" against MySQL 8.0: 54 tests, 46 passed, 0 failed, 8 skipped (option-4-only invariants Assume-skip on options 2/3). Includes new testListAllNullPrefixPagination (options 2/3/4) and testListAllNullPrefixReturnsLatestVersionUnderOption4, plus the existing testListAllNullPrefixHidesDeletedLatestUnderOption4 regression guard (green under the new option-4 LIST_ALL). - ./gradlew :ambry-frontend:intTest --tests "*S3MySqlNamedBlobListIntegrationTest" --tests "*S3IntegrationTest" against MySQL 8.0: 4 passed, 0 failed. The new S3MySqlNamedBlobListIntegrationTest exercises the empty-prefix LIST end-to-end (S3 SDK -> Netty -> S3ListHandler -> parseS3 -> NamedBlobListHandler -> MySqlNamedBlobDb.list null-prefix -> option-4 LIST_ALL) against real MySQL and asserts 200 -- the stitched coverage previously deferred; existing S3IntegrationTest still passes after the buildFrontendVProps refactor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2572752 commit 0e1c73c

5 files changed

Lines changed: 430 additions & 32 deletions

File tree

ambry-api/src/main/java/com/github/ambry/config/MySqlNamedBlobDbConfig.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public class MySqlNamedBlobDbConfig {
3232
public static final String LIST_MAX_RESULTS = PREFIX + "list.max.results";
3333
public static final String QUERY_STALE_DATA_MAX_RESULTS = PREFIX + "query.stale.data.max.results";
3434
public static final String STALE_DATA_RETENTION_DAYS = PREFIX + "stale.data.retention.days";
35+
public static final String LIST_QUERY_TIMEOUT_SECONDS = PREFIX + "list.query.timeout.seconds";
3536
public static final String TRANSACTION_ISOLATION_LEVEL = PREFIX + "transaction.isolation.level";
3637
public static final String LIST_NAMED_BLOBS_SQL_OPTION = "list.named.blobs.sql.option";
3738
public static final String ENABLE_HARD_DELETE = PREFIX + "enable.hard.delete";
@@ -117,6 +118,20 @@ public class MySqlNamedBlobDbConfig {
117118
@Default("5")
118119
public final int staleDataRetentionDays;
119120

121+
/**
122+
* Per-statement timeout (in seconds) applied to LIST queries via {@link java.sql.Statement#setQueryTimeout(int)}.
123+
* Guards against a LIST scanning an unexpectedly large container: when the budget is exceeded the JDBC driver
124+
* issues a clean cancel and the server throws a {@link java.sql.SQLException} (e.g. MySQLTimeoutException),
125+
* which surfaces as an ordinary error rather than tearing the socket ("Communications link failure" -> HTTP 500).
126+
*
127+
* <p><b>Default 0 disables the timeout</b>, making this a no-op for existing deployments. Operators on fabrics
128+
* where a network/socket timeout is shorter than the server statement kill should set this just below that
129+
* socket timeout so a slow LIST fails fast and cleanly instead of poisoning the connection.
130+
*/
131+
@Config(LIST_QUERY_TIMEOUT_SECONDS)
132+
@Default("0")
133+
public final int listQueryTimeoutSeconds;
134+
120135
/**
121136
* Transaction isolation level to be set on DB Connection. When nothing is set, default MySQL DB transaction level
122137
* (REPEATABLE_READ) will take effect.
@@ -164,6 +179,8 @@ public MySqlNamedBlobDbConfig(VerifiableProperties verifiableProperties) {
164179
verifiableProperties.getIntInRange(QUERY_STALE_DATA_MAX_RESULTS, 1000, 1, Integer.MAX_VALUE);
165180
this.staleDataRetentionDays =
166181
verifiableProperties.getIntInRange(STALE_DATA_RETENTION_DAYS, 5, 1, Integer.MAX_VALUE);
182+
this.listQueryTimeoutSeconds =
183+
verifiableProperties.getIntInRange(LIST_QUERY_TIMEOUT_SECONDS, 0, 0, Integer.MAX_VALUE);
167184
this.transactionIsolationLevel =
168185
verifiableProperties.getEnum(TRANSACTION_ISOLATION_LEVEL, TransactionIsolationLevel.class,
169186
TransactionIsolationLevel.TRANSACTION_NONE);

ambry-frontend/src/integration-test/java/com/github/ambry/frontend/S3IntegrationTest.java

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -392,9 +392,25 @@ public void s3ListEmptyPrefixTest() throws Exception {
392392
* @param account {@link Account} for which quota needs to be specified.
393393
* @return a {@link VerifiableProperties} with the parameters for an Ambry frontend server.
394394
*/
395-
private static VerifiableProperties buildFrontendVPropsForQuota(File trustStoreFile, Account account)
395+
static VerifiableProperties buildFrontendVPropsForQuota(File trustStoreFile, Account account)
396396
throws IOException, GeneralSecurityException {
397-
Properties properties = buildFrontendVProps(trustStoreFile);
397+
return buildFrontendVPropsForQuota(trustStoreFile, account, "com.github.ambry.commons.InMemNamedBlobDbFactory",
398+
null);
399+
}
400+
401+
/**
402+
* Builds quota-enabled frontend properties, letting the caller pick the named-blob DB factory (e.g. the
403+
* MySQL-backed factory) and supply extra properties (e.g. the dbInfo and LIST SQL option). Reused by
404+
* sibling integration tests that exercise the S3 stack against a real backend.
405+
* @param trustStoreFile the trust store file to add certificates to for SSL testing.
406+
* @param account {@link Account} for which quota needs to be specified.
407+
* @param namedBlobDbFactory the fully-qualified {@link com.github.ambry.named.NamedBlobDbFactory} class name.
408+
* @param extraProps additional properties to layer on top (may be null).
409+
* @return a {@link VerifiableProperties} with the parameters for an Ambry frontend server.
410+
*/
411+
static VerifiableProperties buildFrontendVPropsForQuota(File trustStoreFile, Account account,
412+
String namedBlobDbFactory, Properties extraProps) throws IOException, GeneralSecurityException {
413+
Properties properties = buildFrontendVProps(trustStoreFile, namedBlobDbFactory, extraProps);
398414
JSONObject cuResourceQuotaJson = new JSONObject();
399415
JSONObject quotaJson = new JSONObject();
400416
quotaJson.put("rcu", 10737418240L);
@@ -413,7 +429,18 @@ private static VerifiableProperties buildFrontendVPropsForQuota(File trustStoreF
413429
* @param trustStoreFile the trust store file to add certificates to for SSL testing.
414430
* @return a {@link Properties} with the parameters for an Ambry frontend server.
415431
*/
416-
private static Properties buildFrontendVProps(File trustStoreFile)
432+
static Properties buildFrontendVProps(File trustStoreFile) throws IOException, GeneralSecurityException {
433+
return buildFrontendVProps(trustStoreFile, "com.github.ambry.commons.InMemNamedBlobDbFactory", null);
434+
}
435+
436+
/**
437+
* Builds frontend properties with a caller-selected named-blob DB factory and optional extra properties.
438+
* @param trustStoreFile the trust store file to add certificates to for SSL testing.
439+
* @param namedBlobDbFactory the fully-qualified {@link com.github.ambry.named.NamedBlobDbFactory} class name.
440+
* @param extraProps additional properties to layer on top (may be null).
441+
* @return a {@link Properties} with the parameters for an Ambry frontend server.
442+
*/
443+
static Properties buildFrontendVProps(File trustStoreFile, String namedBlobDbFactory, Properties extraProps)
417444
throws IOException, GeneralSecurityException {
418445
Properties properties = new Properties();
419446
properties.put("rest.server.rest.request.service.factory",
@@ -436,8 +463,11 @@ private static Properties buildFrontendVProps(File trustStoreFile)
436463
properties.setProperty("clustermap.datacenter.name", DATA_CENTER_NAME);
437464
properties.setProperty("clustermap.host.name", HOST_NAME);
438465
properties.setProperty(FrontendConfig.ENABLE_UNDELETE, Boolean.toString(true));
439-
properties.setProperty(FrontendConfig.NAMED_BLOB_DB_FACTORY, "com.github.ambry.commons.InMemNamedBlobDbFactory");
466+
properties.setProperty(FrontendConfig.NAMED_BLOB_DB_FACTORY, namedBlobDbFactory);
440467
properties.setProperty(MySqlNamedBlobDbConfig.LIST_MAX_RESULTS, String.valueOf(NAMED_BLOB_LIST_RESULT_MAX));
468+
if (extraProps != null) {
469+
properties.putAll(extraProps);
470+
}
441471
return properties;
442472
}
443473
}
Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
/**
2+
* Copyright 2026 LinkedIn Corp. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
*/
14+
package com.github.ambry.frontend;
15+
16+
import com.github.ambry.account.Account;
17+
import com.github.ambry.account.Container;
18+
import com.github.ambry.account.InMemAccountService;
19+
import com.github.ambry.account.InMemAccountServiceFactory;
20+
import com.github.ambry.clustermap.MockClusterMap;
21+
import com.github.ambry.commons.LoggingNotificationSystem;
22+
import com.github.ambry.commons.SSLFactory;
23+
import com.github.ambry.commons.TestSSLUtils;
24+
import com.github.ambry.config.FrontendConfig;
25+
import com.github.ambry.config.MySqlNamedBlobDbConfig;
26+
import com.github.ambry.config.SSLConfig;
27+
import com.github.ambry.config.VerifiableProperties;
28+
import com.github.ambry.quota.QuotaResourceType;
29+
import com.github.ambry.rest.NettyClient;
30+
import com.github.ambry.rest.RestServer;
31+
import com.github.ambry.utils.TestUtils;
32+
import io.netty.handler.codec.http.DefaultHttpHeaders;
33+
import io.netty.handler.codec.http.FullHttpRequest;
34+
import io.netty.handler.codec.http.HttpHeaders;
35+
import io.netty.handler.codec.http.HttpMethod;
36+
import io.netty.handler.codec.http.HttpResponse;
37+
import io.netty.handler.codec.http.HttpResponseStatus;
38+
import java.io.File;
39+
import java.nio.ByteBuffer;
40+
import java.sql.Connection;
41+
import java.sql.DriverManager;
42+
import java.sql.SQLException;
43+
import java.sql.Statement;
44+
import java.util.Collections;
45+
import java.util.Properties;
46+
import org.junit.AfterClass;
47+
import org.junit.Assume;
48+
import org.junit.Before;
49+
import org.junit.BeforeClass;
50+
import org.junit.Test;
51+
52+
import static com.github.ambry.rest.RestUtils.Headers.*;
53+
import static com.github.ambry.rest.RestUtils.*;
54+
import static org.junit.Assert.*;
55+
56+
57+
/**
58+
* Integration test that exercises the S3 empty-prefix LIST path end-to-end against a REAL MySQL backend under
59+
* {@code listNamedBlobsSqlOption=4}: HTTP -> Netty -> S3ListHandler -> NamedBlobPath.parseS3 (empty prefix collapses
60+
* to null) -> NamedBlobListHandler -> {@link com.github.ambry.named.MySqlNamedBlobDb#list} -> LIST_ALL (option 4).
61+
*
62+
* <p>This is the stitched coverage that {@code S3IntegrationTest#s3ListEmptyPrefixTest} could not provide (it uses
63+
* {@code InMemNamedBlobDbFactory}, so it never runs the SQL), and that the SQL-only int test
64+
* {@code MySqlNamedBlobDbListOperationIntegrationTest} could not provide (it never goes through the S3 handler). It is
65+
* the only layer that runs the exact path that returned HTTP 500 on large containers when LIST_ALL option 4 used a
66+
* window-function plan that materialized the whole container.
67+
*
68+
* <p>The CI {@code int-test} job provisions {@code localhost/AmbryNamedBlobs} (NamedBlobsSchema) and the {@code travis}
69+
* user, so this test runs there. When no such MySQL is reachable (e.g. local dev), it is skipped via {@link Assume}.
70+
*/
71+
public class S3MySqlNamedBlobListIntegrationTest extends FrontendIntegrationTestBase {
72+
// dbInfo "datacenter" MUST match clustermap.datacenter.name that S3IntegrationTest.buildFrontendVProps sets.
73+
private static final String DATA_CENTER_NAME = "localDc";
74+
private static final String NAMED_BLOB_DB_URL = "jdbc:mysql://localhost/AmbryNamedBlobs?serverTimezone=UTC";
75+
private static final String NAMED_BLOB_DB_USER = "travis";
76+
private static final String NAMED_BLOB_DB_PASSWORD = "";
77+
private static final String DB_INFO =
78+
"[{\"url\":\"" + NAMED_BLOB_DB_URL + "\",\"datacenter\":\"" + DATA_CENTER_NAME
79+
+ "\",\"isWriteable\":\"true\",\"username\":\"" + NAMED_BLOB_DB_USER + "\",\"password\":\""
80+
+ NAMED_BLOB_DB_PASSWORD + "\",\"sslMode\":\"NONE\"}]";
81+
82+
private static final MockClusterMap CLUSTER_MAP;
83+
private static final VerifiableProperties FRONTEND_VERIFIABLE_PROPS;
84+
private static final VerifiableProperties SSL_CLIENT_VERIFIABLE_PROPS;
85+
private static final FrontendConfig FRONTEND_CONFIG;
86+
// InMemAccountServiceFactory returns a (returnOnlyUnknown, notifyConsumers)-keyed singleton, so this is the SAME
87+
// instance the frontend's account service resolves against -> the account created here is visible to MySqlNamedBlobDb.
88+
private static final InMemAccountService ACCOUNT_SERVICE =
89+
new InMemAccountServiceFactory(false, true).getAccountService();
90+
private static final Account ACCOUNT;
91+
private static RestServer ambryRestServer = null;
92+
private static NettyClient plaintextNettyClient = null;
93+
private static NettyClient sslNettyClient = null;
94+
private static boolean mysqlAvailable = false;
95+
96+
static {
97+
try {
98+
CLUSTER_MAP = new MockClusterMap();
99+
File trustStoreFile = File.createTempFile("truststore", ".jks");
100+
trustStoreFile.deleteOnExit();
101+
SSL_CLIENT_VERIFIABLE_PROPS = TestSSLUtils.createSslProps("", SSLFactory.Mode.CLIENT, trustStoreFile, "client");
102+
ACCOUNT_SERVICE.clear();
103+
ACCOUNT_SERVICE.updateAccounts(Collections.singletonList(InMemAccountService.UNKNOWN_ACCOUNT));
104+
ACCOUNT = ACCOUNT_SERVICE.createAndAddRandomAccount(QuotaResourceType.ACCOUNT);
105+
Properties mysqlProps = new Properties();
106+
mysqlProps.setProperty(MySqlNamedBlobDbConfig.DB_INFO, DB_INFO);
107+
// Exercise the option-4 LIST_ALL path end-to-end (the empty-prefix -> null-prefix LIST that 500'd on large containers).
108+
mysqlProps.setProperty(MySqlNamedBlobDbConfig.LIST_NAMED_BLOBS_SQL_OPTION,
109+
Integer.toString(MySqlNamedBlobDbConfig.MAX_LIST_NAMED_BLOBS_SQL_OPTION));
110+
FRONTEND_VERIFIABLE_PROPS = S3IntegrationTest.buildFrontendVPropsForQuota(trustStoreFile, ACCOUNT,
111+
"com.github.ambry.named.MySqlNamedBlobDbFactory", mysqlProps);
112+
FRONTEND_CONFIG = new FrontendConfig(FRONTEND_VERIFIABLE_PROPS);
113+
} catch (Throwable t) {
114+
throw new IllegalStateException(t);
115+
}
116+
}
117+
118+
public S3MySqlNamedBlobListIntegrationTest() {
119+
super(FRONTEND_CONFIG, sslNettyClient);
120+
}
121+
122+
@BeforeClass
123+
public static void setup() throws Exception {
124+
// Skip cleanly when no int-test MySQL is reachable (e.g. local dev without it). CI's int-test job provisions
125+
// localhost/AmbryNamedBlobs with the NamedBlobsSchema, so the test runs there.
126+
try (Connection ignored = DriverManager.getConnection(NAMED_BLOB_DB_URL, NAMED_BLOB_DB_USER,
127+
NAMED_BLOB_DB_PASSWORD)) {
128+
mysqlAvailable = true;
129+
} catch (SQLException e) {
130+
Assume.assumeNoException("MySQL (localhost/AmbryNamedBlobs) not reachable; skipping MySQL-backed S3 LIST test",
131+
e);
132+
}
133+
ambryRestServer = new RestServer(FRONTEND_VERIFIABLE_PROPS, CLUSTER_MAP, new LoggingNotificationSystem(),
134+
SSLFactory.getNewInstance(new SSLConfig(FRONTEND_VERIFIABLE_PROPS)));
135+
ambryRestServer.start();
136+
plaintextNettyClient = new NettyClient("localhost", PLAINTEXT_SERVER_PORT, null);
137+
sslNettyClient = new NettyClient("localhost", SSL_SERVER_PORT,
138+
SSLFactory.getNewInstance(new SSLConfig(SSL_CLIENT_VERIFIABLE_PROPS)));
139+
}
140+
141+
@AfterClass
142+
public static void teardown() {
143+
if (plaintextNettyClient != null) {
144+
plaintextNettyClient.close();
145+
}
146+
if (sslNettyClient != null) {
147+
sslNettyClient.close();
148+
}
149+
if (ambryRestServer != null) {
150+
ambryRestServer.shutdown();
151+
}
152+
}
153+
154+
/**
155+
* Clears any rows left from a prior run so the empty-prefix LIST assertions are deterministic across reruns.
156+
*/
157+
@Before
158+
public void before() throws Exception {
159+
Assume.assumeTrue(mysqlAvailable);
160+
this.nettyClient = sslNettyClient;
161+
Container container = ACCOUNT.getAllContainers().iterator().next();
162+
try (Connection connection = DriverManager.getConnection(NAMED_BLOB_DB_URL, NAMED_BLOB_DB_USER,
163+
NAMED_BLOB_DB_PASSWORD); Statement statement = connection.createStatement()) {
164+
statement.executeUpdate(String.format("DELETE FROM named_blobs_v2 WHERE account_id = %d AND container_id = %d",
165+
ACCOUNT.getId(), container.getId()));
166+
}
167+
}
168+
169+
/**
170+
* PUT a few named blobs through the S3 stack, then issue an empty-prefix LIST (both v1 and v2 shapes) and assert
171+
* 200 OK end-to-end. With {@code listNamedBlobsSqlOption=4}, an empty prefix collapses to a null prefix and runs the
172+
* correlated-subquery LIST_ALL form against real MySQL. A regression in the S3-handler routing, the empty-prefix
173+
* collapse, or the option-4 LIST_ALL SQL/param binding surfaces here as a non-200 (or a 500 on the window plan).
174+
*/
175+
@Test
176+
public void s3EmptyPrefixListAgainstMySqlOption4Test() throws Exception {
177+
String account = ACCOUNT.getName();
178+
Container container = ACCOUNT.getAllContainers().iterator().next();
179+
String containerName = container.getName();
180+
181+
String[] keys = new String[]{"empty_prefix_mysql_a", "empty_prefix_mysql_b", "empty_prefix_mysql_c"};
182+
int contentSize = 64;
183+
for (String key : keys) {
184+
doPutBlob(account, containerName, key, contentSize, TestUtils.getRandomBytes(contentSize));
185+
}
186+
187+
// V1 LIST: GET /s3/{account}/{container}?prefix= (explicit empty value)
188+
String uriV1 = String.format("/s3/%s/%s?prefix=", account, containerName);
189+
FullHttpRequest reqV1 = buildRequest(HttpMethod.GET, uriV1, new DefaultHttpHeaders(), null);
190+
HttpResponse respV1 = getHttpResponse(nettyClient.sendRequest(reqV1, null, null).get());
191+
assertEquals("Empty-prefix LIST (v1) through the S3 stack against MySQL option-4 LIST_ALL must return 200 OK",
192+
HttpResponseStatus.OK, respV1.status());
193+
194+
// V2 LIST: GET /s3/{account}/{container}?prefix=&list-type=2
195+
String uriV2 = String.format("/s3/%s/%s?prefix=&list-type=2", account, containerName);
196+
FullHttpRequest reqV2 = buildRequest(HttpMethod.GET, uriV2, new DefaultHttpHeaders(), null);
197+
HttpResponse respV2 = getHttpResponse(nettyClient.sendRequest(reqV2, null, null).get());
198+
assertEquals("Empty-prefix LIST (v2) through the S3 stack against MySQL option-4 LIST_ALL must return 200 OK",
199+
HttpResponseStatus.OK, respV2.status());
200+
}
201+
202+
private void doPutBlob(String account, String container, String key, int contentSize, byte[] content)
203+
throws Exception {
204+
String uri = String.format("/s3/%s/%s/%s", account, container, key);
205+
HttpHeaders headers = new DefaultHttpHeaders();
206+
headers.add(CONTENT_TYPE, OCTET_STREAM_CONTENT_TYPE);
207+
headers.add(CONTENT_LENGTH, contentSize);
208+
FullHttpRequest httpRequest = buildRequest(HttpMethod.PUT, uri, headers, ByteBuffer.wrap(content));
209+
HttpResponse response = getHttpResponse(nettyClient.sendRequest(httpRequest, null, null).get());
210+
assertEquals("Unexpected status putting seed blob " + key, HttpResponseStatus.OK, response.status());
211+
}
212+
}

0 commit comments

Comments
 (0)