Skip to content

Commit 5c910d7

Browse files
kvarghaCopilot
andauthored
[fast-client] Change defaults to production values (linkedin#2895)
Enable long-tail retry by default for single-get and batch-get, matching the values run in production: - longTailRetryEnabledForSingleGet: false -> true - longTailRetryEnabledForBatchGet: false -> true Thresholds are unchanged because they already match production (single-get 1ms; batch-get falls back to the existing range-based threshold map when the fixed threshold is 0). Compute long-tail retry stays disabled, also matching production. The behavior is bounded by the retry budget, which is already enabled by default (10%). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2e7b08d commit 5c910d7

3 files changed

Lines changed: 23 additions & 9 deletions

File tree

clients/venice-client/src/main/java/com/linkedin/venice/fastclient/ClientConfig.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,10 +449,10 @@ public static class ClientConfigBuilder<K, V, T extends SpecificRecord> {
449449
private long metadataRefreshIntervalInSeconds = -1;
450450
private long metadataConnWarmupTimeoutInSeconds = -1;
451451

452-
private boolean longTailRetryEnabledForSingleGet = false;
452+
private boolean longTailRetryEnabledForSingleGet = true;
453453
private int longTailRetryThresholdForSingleGetInMicroSeconds = 1000; // 1ms.
454454

455-
private boolean longTailRetryEnabledForBatchGet = false;
455+
private boolean longTailRetryEnabledForBatchGet = true;
456456
private int longTailRetryThresholdForBatchGetInMicroSeconds = 0;
457457

458458
private String longTailRangeBasedRetryThresholdForBatchGetInMilliSeconds =

internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/fastclient/AvroStoreClientEndToEndTest.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,9 @@ public void testFastClientGet(
209209
ClientConfig.ClientConfigBuilder clientConfigBuilder =
210210
new ClientConfig.ClientConfigBuilder<>().setStoreName(storeName)
211211
.setR2Client(r2Client)
212-
.setDualReadEnabled(dualRead);
212+
.setDualReadEnabled(dualRead)
213+
.setLongTailRetryEnabledForSingleGet(false)
214+
.setLongTailRetryEnabledForBatchGet(false);
213215
// Test HAR algorithm in this test.
214216
Set<String> harClusters = new HashSet<>();
215217
harClusters.add(veniceCluster.getServerD2ServiceName());
@@ -314,7 +316,9 @@ public void testFastClientGetWithDifferentHTTPVariants(
314316
ClientConfig.ClientConfigBuilder clientConfigBuilder =
315317
new ClientConfig.ClientConfigBuilder<>().setStoreName(storeName)
316318
.setR2Client(r2Client)
317-
.setDualReadEnabled(false);
319+
.setDualReadEnabled(false)
320+
.setLongTailRetryEnabledForSingleGet(false)
321+
.setLongTailRetryEnabledForBatchGet(false);
318322
// single get
319323
Consumer<MetricsRepository> fastClientStatsValidation =
320324
metricsRepository -> validateSingleGetMetrics(metricsRepository, false, false);

internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/fastclient/BatchGetAvroStoreClientTest.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ public void testBatchGetGenericClient(boolean retryEnabled, StoreMetadataFetchMo
131131
ClientConfig.ClientConfigBuilder clientConfigBuilder =
132132
new ClientConfig.ClientConfigBuilder<>().setStoreName(storeName)
133133
.setR2Client(r2Client)
134-
.setDualReadEnabled(false);
134+
.setDualReadEnabled(false)
135+
.setLongTailRetryEnabledForSingleGet(false)
136+
.setLongTailRetryEnabledForBatchGet(false);
135137

136138
if (retryEnabled) {
137139
// enable retry to test the code path: to mimic retry in integration tests
@@ -169,7 +171,9 @@ public void testBatchGetSpecificClient(StoreMetadataFetchMode storeMetadataFetch
169171
ClientConfig.ClientConfigBuilder clientConfigBuilder =
170172
new ClientConfig.ClientConfigBuilder<>().setStoreName(storeName)
171173
.setR2Client(r2Client)
172-
.setDualReadEnabled(false);
174+
.setDualReadEnabled(false)
175+
.setLongTailRetryEnabledForSingleGet(false)
176+
.setLongTailRetryEnabledForBatchGet(false);
173177

174178
VeniceMetricsRepository metricsRepository = createVeniceMetricsRepository(true);
175179
AvroSpecificStoreClient<String, TestValueSchema> specificFastClient =
@@ -199,7 +203,9 @@ public void testStreamingBatchGetGenericClient(boolean retryEnabled, StoreMetada
199203
ClientConfig.ClientConfigBuilder clientConfigBuilder =
200204
new ClientConfig.ClientConfigBuilder<>().setStoreName(storeName)
201205
.setR2Client(r2Client)
202-
.setDualReadEnabled(false);
206+
.setDualReadEnabled(false)
207+
.setLongTailRetryEnabledForSingleGet(false)
208+
.setLongTailRetryEnabledForBatchGet(false);
203209

204210
if (retryEnabled) {
205211
// enable retry to test the code path: to mimic retry in integration tests
@@ -257,7 +263,9 @@ public void testStreamingBatchGetLimit() throws Exception {
257263
ClientConfig.ClientConfigBuilder clientConfigBuilder =
258264
new ClientConfig.ClientConfigBuilder<>().setStoreName(storeName)
259265
.setR2Client(r2Client)
260-
.setDualReadEnabled(false);
266+
.setDualReadEnabled(false)
267+
.setLongTailRetryEnabledForSingleGet(false)
268+
.setLongTailRetryEnabledForBatchGet(false);
261269

262270
VeniceMetricsRepository metricsRepository = createVeniceMetricsRepository(true);
263271
AvroGenericStoreClient<String, GenericRecord> genericFastClient =
@@ -301,7 +309,9 @@ public void testStreamingBatchGetWithCallbackGenericClient(
301309
ClientConfig.ClientConfigBuilder clientConfigBuilder =
302310
new ClientConfig.ClientConfigBuilder<>().setStoreName(storeName)
303311
.setR2Client(r2Client)
304-
.setDualReadEnabled(false);
312+
.setDualReadEnabled(false)
313+
.setLongTailRetryEnabledForSingleGet(false)
314+
.setLongTailRetryEnabledForBatchGet(false);
305315

306316
if (retryEnabled) {
307317
// enable retry to test the code path: to mimic retry in integration tests

0 commit comments

Comments
 (0)