Skip to content

Commit

Permalink
cleanup RetriableAvroGenericStoreClientTest
Browse files Browse the repository at this point in the history
  • Loading branch information
@m-nagarajan
m-nagarajan committed Jul 21, 2023
1 parent 1570034 commit dbd8ccf
Showing 1 changed file with 149 additions and 146 deletions.
295 changes: 149 additions & 146 deletions ...ent/src/test/java/com/linkedin/venice/fastclient/RetriableAvroGenericStoreClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,139 @@ protected CompletableFuture<VeniceResponseMap> streamingBatchGet(
return metrics;
}

/**
* @param bothOriginalAndRetryFails In this case, get and batchGet both will throw veniceClientException
* and most metrics are not incremented, but requestContext values should
* be checked. Check {@link StatsAvroGenericStoreClient#recordRequestMetrics}
* for more details.
* @param errorRetry request is retried because the original request results in exception
* @param longTailRetry request is retried because the original request is taking more time
* @param retryWin retry request wins
*/
private void testSingleGetAndValidateMetrics(
boolean bothOriginalAndRetryFails,
boolean errorRetry,
boolean longTailRetry,
boolean retryWin) throws ExecutionException, InterruptedException {
getRequestContext = new GetRequestContext();
try {
String value = (String) statsAvroGenericStoreClient.get(getRequestContext, "test_key").get();
if (bothOriginalAndRetryFails) {
fail("An ExecutionException should be thrown here");
}
assertEquals(value, SINGLE_GET_VALUE_RESPONSE);
} catch (ExecutionException e) {
if (!bothOriginalAndRetryFails) {
throw e;
}
}

metrics = getStats(clientConfig);
assertEquals(metrics.get("." + STORE_NAME + "--request_key_count.Max").value(), 1.0);

if (errorRetry || longTailRetry) {
if (!bothOriginalAndRetryFails) {
assertEquals(metrics.get("." + STORE_NAME + "--retry_request_key_count.Max").value(), 1.0);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--retry_request_key_count.Max").value() > 0);
}
} else {
assertFalse(metrics.get("." + STORE_NAME + "--retry_request_key_count.Max").value() > 0);
}
if (errorRetry) {
if (!bothOriginalAndRetryFails) {
assertTrue(metrics.get("." + STORE_NAME + "--error_retry_request.OccurrenceRate").value() > 0);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--error_retry_request.OccurrenceRate").value() > 0);
}
assertTrue(getRequestContext.errorRetryRequestTriggered);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--error_retry_request.OccurrenceRate").value() > 0);
assertFalse(getRequestContext.errorRetryRequestTriggered);
}

if (longTailRetry) {
if (!bothOriginalAndRetryFails) {
assertTrue(metrics.get("." + STORE_NAME + "--long_tail_retry_request.OccurrenceRate").value() > 0);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--long_tail_retry_request.OccurrenceRate").value() > 0);
}
assertTrue(getRequestContext.longTailRetryRequestTriggered);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--long_tail_retry_request.OccurrenceRate").value() > 0);
assertFalse(getRequestContext.longTailRetryRequestTriggered);
}

if (retryWin) {
assertTrue(metrics.get("." + STORE_NAME + "--retry_request_win.OccurrenceRate").value() > 0);
assertEquals(metrics.get("." + STORE_NAME + "--retry_request_success_key_count.Max").value(), 1.0);
assertTrue(getRequestContext.retryWin);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--retry_request_win.OccurrenceRate").value() > 0);
assertFalse(metrics.get("." + STORE_NAME + "--retry_request_success_key_count.Max").value() > 0);
assertFalse(getRequestContext.retryWin);
}
}

/**
* @param bothOriginalAndRetryFails In this case, get and batchGet both will throw veniceClientException
* and most metrics are not incremented, but requestContext values should
* be checked. Check {@link StatsAvroGenericStoreClient#recordRequestMetrics}
* for more details.
* @param longTailRetry request is retried because the original request is taking more time
* @param retryWin retry request wins
*/
private void testBatchGetAndvalidateMetrics(
boolean bothOriginalAndRetryFails,
boolean longTailRetry,
boolean retryWin) throws ExecutionException, InterruptedException {
batchGetRequestContext = new BatchGetRequestContext<>();
try {
Map<String, String> value =
(Map<String, String>) statsAvroGenericStoreClient.batchGet(batchGetRequestContext, BATCH_GET_KEYS).get();

if (bothOriginalAndRetryFails) {
fail("An ExecutionException should be thrown here");
}
assertEquals(value, BATCH_GET_VALUE_RESPONSE);
} catch (ExecutionException e) {
if (!bothOriginalAndRetryFails) {
throw e;
}
}
metrics = getStats(clientConfig, RequestType.MULTI_GET);

assertEquals(metrics.get("." + STORE_NAME + "--multiget_request_key_count.Max").value(), 2.0);

if (longTailRetry) {
if (!bothOriginalAndRetryFails) {
assertTrue(metrics.get("." + STORE_NAME + "--multiget_long_tail_retry_request.OccurrenceRate").value() > 0);
assertTrue(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Rate").value() > 0);
assertEquals(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Max").value(), 2.0);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--multiget_long_tail_retry_request.OccurrenceRate").value() > 0);
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Rate").value() > 0);
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Max").value() > 0);
}
assertTrue(batchGetRequestContext.longTailRetryTriggered);
assertEquals(batchGetRequestContext.numberOfKeysSentInRetryRequest, 2);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--multiget_long_tail_retry_request.OccurrenceRate").value() > 0);
assertFalse(batchGetRequestContext.longTailRetryTriggered);
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Rate").value() > 0);
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Max").value() > 0);
assertFalse(batchGetRequestContext.numberOfKeysSentInRetryRequest > 0);
}

if (retryWin) {
assertTrue(metrics.get("." + STORE_NAME + "--multiget_retry_request_success_key_count.Rate").value() > 0);
assertEquals(batchGetRequestContext.numberOfKeysCompletedInRetryRequest.get(), 2);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_success_key_count.Rate").value() > 0);
assertFalse(batchGetRequestContext.numberOfKeysCompletedInRetryRequest.get() > 0);
}
}

/**
* Original request is faster than retry threshold.
*/
Expand All @@ -202,9 +335,9 @@ public void testGetWithoutTriggeringLongTailRetry(boolean batchGet) throws Execu
clientConfig);
statsAvroGenericStoreClient = new StatsAvroGenericStoreClient(retriableClient, clientConfig);
if (!batchGet) {
validateSingleGetMetrics(false, false, false, false);
testSingleGetAndValidateMetrics(false, false, false, false);
} else {
validateBatchGetMetrics(false, false, false);
testBatchGetAndvalidateMetrics(false, false, false);
}
}

Expand All @@ -226,9 +359,9 @@ public void testGetWithTriggeringLongTailRetryAndOriginalWins(boolean batchGet)
clientConfig);
statsAvroGenericStoreClient = new StatsAvroGenericStoreClient(retriableClient, clientConfig);
if (!batchGet) {
validateSingleGetMetrics(false, false, true, false);
testSingleGetAndValidateMetrics(false, false, true, false);
} else {
validateBatchGetMetrics(false, true, false);
testBatchGetAndvalidateMetrics(false, true, false);
}
}

Expand All @@ -250,9 +383,9 @@ public void testGetWithTriggeringLongTailRetryAndRetryWins(boolean batchGet)
clientConfig);
statsAvroGenericStoreClient = new StatsAvroGenericStoreClient(retriableClient, clientConfig);
if (!batchGet) {
validateSingleGetMetrics(false, false, true, true);
testSingleGetAndValidateMetrics(false, false, true, true);
} else {
validateBatchGetMetrics(false, true, true);
testBatchGetAndvalidateMetrics(false, true, true);
}
}

Expand All @@ -269,9 +402,9 @@ public void testGetWithTriggeringErrorRetryAndRetryWins(boolean batchGet)
clientConfig);
statsAvroGenericStoreClient = new StatsAvroGenericStoreClient(retriableClient, clientConfig);
if (!batchGet) {
validateSingleGetMetrics(false, true, false, true);
testSingleGetAndValidateMetrics(false, true, false, true);
} else {
validateBatchGetMetrics(false, true, true);
testBatchGetAndvalidateMetrics(false, true, true);
}
}

Expand All @@ -288,9 +421,9 @@ public void testGetWithTriggeringLongTailRetryAndRetryFails(boolean batchGet)
clientConfig);
statsAvroGenericStoreClient = new StatsAvroGenericStoreClient(retriableClient, clientConfig);
if (!batchGet) {
validateSingleGetMetrics(false, false, true, false);
testSingleGetAndValidateMetrics(false, false, true, false);
} else {
validateBatchGetMetrics(false, true, false);
testBatchGetAndvalidateMetrics(false, true, false);
}
}

Expand All @@ -313,18 +446,19 @@ public void testGetWithTriggeringLongTailRetryAndBothFailsV1(boolean batchGet)
* Check {@link StatsAvroGenericStoreClient#recordRequestMetrics} for more details.
*/
if (!batchGet) {
validateSingleGetMetrics(true, false, true, false);
testSingleGetAndValidateMetrics(true, false, true, false);
} else {
validateBatchGetMetrics(true, true, false);
testBatchGetAndvalidateMetrics(true, true, false);
}
}

/**
* Original request latency is lower than the retry threshold, and both the original request and the retry fails.
*/
@Test(dataProvider = "True-and-False", dataProviderClass = DataProviderUtils.class, timeOut = TEST_TIMEOUT)
@Test(dataProvider = "True-and-False", dataProviderClass = DataProviderUtils.class/*, timeOut = TEST_TIMEOUT*/)
public void testGetWithTriggeringLongTailRetryAndBothFailsV2(boolean batchGet)
throws InterruptedException, ExecutionException {
batchGet = true;
clientConfigBuilder.setMetricsRepository(new MetricsRepository());
clientConfig = clientConfigBuilder.build();
retriableClient =
Expand All @@ -337,140 +471,9 @@ public void testGetWithTriggeringLongTailRetryAndBothFailsV2(boolean batchGet)
* Check {@link StatsAvroGenericStoreClient#recordRequestMetrics} for more details.
*/
if (!batchGet) {
validateSingleGetMetrics(true, true, false, false);
} else {
validateBatchGetMetrics(true, true, false);
}
}

/**
* @param bothOriginalAndRetryFails In this case, get and batchGet both will throw veniceClientException
* and most metrics are not incremented, but requestContext values should
* be checked. Check {@link StatsAvroGenericStoreClient#recordRequestMetrics}
* for more details.
* @param errorRetry request is retried because the original request results in exception
* @param longTailRetry request is retried because the original request is taking more time
* @param retryWin retry request wins
*/
private void validateSingleGetMetrics(
boolean bothOriginalAndRetryFails,
boolean errorRetry,
boolean longTailRetry,
boolean retryWin) throws ExecutionException, InterruptedException {
getRequestContext = new GetRequestContext();
try {
String value = (String) statsAvroGenericStoreClient.get(getRequestContext, "test_key").get();
if (bothOriginalAndRetryFails) {
fail("An ExecutionException should be thrown here");
}
assertEquals(value, SINGLE_GET_VALUE_RESPONSE);
} catch (ExecutionException e) {
if (!bothOriginalAndRetryFails) {
throw e;
}
}

metrics = getStats(clientConfig);
assertEquals(metrics.get("." + STORE_NAME + "--request_key_count.Max").value(), 1.0);

if (errorRetry || longTailRetry) {
if (!bothOriginalAndRetryFails) {
assertEquals(metrics.get("." + STORE_NAME + "--retry_request_key_count.Max").value(), 1.0);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--retry_request_key_count.Max").value() > 0);
}
} else {
assertFalse(metrics.get("." + STORE_NAME + "--retry_request_key_count.Max").value() > 0);
}
if (errorRetry) {
if (!bothOriginalAndRetryFails) {
assertTrue(metrics.get("." + STORE_NAME + "--error_retry_request.OccurrenceRate").value() > 0);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--error_retry_request.OccurrenceRate").value() > 0);
}
assertTrue(getRequestContext.errorRetryRequestTriggered);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--error_retry_request.OccurrenceRate").value() > 0);
assertFalse(getRequestContext.errorRetryRequestTriggered);
}

if (longTailRetry) {
if (!bothOriginalAndRetryFails) {
assertTrue(metrics.get("." + STORE_NAME + "--long_tail_retry_request.OccurrenceRate").value() > 0);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--long_tail_retry_request.OccurrenceRate").value() > 0);
}
assertTrue(getRequestContext.longTailRetryRequestTriggered);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--long_tail_retry_request.OccurrenceRate").value() > 0);
assertFalse(getRequestContext.longTailRetryRequestTriggered);
}

if (retryWin) {
assertTrue(metrics.get("." + STORE_NAME + "--retry_request_win.OccurrenceRate").value() > 0);
assertEquals(metrics.get("." + STORE_NAME + "--retry_request_success_key_count.Max").value(), 1.0);
assertTrue(getRequestContext.retryWin);
testSingleGetAndValidateMetrics(true, true, false, false);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--retry_request_win.OccurrenceRate").value() > 0);
assertFalse(metrics.get("." + STORE_NAME + "--retry_request_success_key_count.Max").value() > 0);
assertFalse(getRequestContext.retryWin);
}
}

/**
* @param bothOriginalAndRetryFails In this case, get and batchGet both will throw veniceClientException
* and most metrics are not incremented, but requestContext values should
* be checked. Check {@link StatsAvroGenericStoreClient#recordRequestMetrics}
* for more details.
* @param longTailRetry request is retried because the original request is taking more time
* @param retryWin retry request wins
*/
private void validateBatchGetMetrics(boolean bothOriginalAndRetryFails, boolean longTailRetry, boolean retryWin)
throws ExecutionException, InterruptedException {
batchGetRequestContext = new BatchGetRequestContext<>();
try {
Map<String, String> value =
(Map<String, String>) statsAvroGenericStoreClient.batchGet(batchGetRequestContext, BATCH_GET_KEYS).get();

if (bothOriginalAndRetryFails) {
fail("An ExecutionException should be thrown here");
}
assertEquals(value, BATCH_GET_VALUE_RESPONSE);
} catch (ExecutionException e) {
if (!bothOriginalAndRetryFails) {
throw e;
}
}
metrics = getStats(clientConfig, RequestType.MULTI_GET);

assertEquals(metrics.get("." + STORE_NAME + "--multiget_request_key_count.Max").value(), 2.0);

if (longTailRetry) {
if (!bothOriginalAndRetryFails) {
assertTrue(metrics.get("." + STORE_NAME + "--multiget_long_tail_retry_request.OccurrenceRate").value() > 0);
assertTrue(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Rate").value() > 0);
assertEquals(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Max").value(), 2.0);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--multiget_long_tail_retry_request.OccurrenceRate").value() > 0);
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Rate").value() > 0);
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Max").value() > 0);
}
assertTrue(batchGetRequestContext.longTailRetryTriggered);
assertEquals(batchGetRequestContext.numberOfKeysSentInRetryRequest, 2);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--multiget_long_tail_retry_request.OccurrenceRate").value() > 0);
assertFalse(batchGetRequestContext.longTailRetryTriggered);
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Rate").value() > 0);
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_key_count.Max").value() > 0);
assertFalse(batchGetRequestContext.numberOfKeysSentInRetryRequest > 0);
}

if (retryWin) {
assertTrue(metrics.get("." + STORE_NAME + "--multiget_retry_request_success_key_count.Rate").value() > 0);
assertEquals(batchGetRequestContext.numberOfKeysCompletedInRetryRequest.get(), 2);
} else {
assertFalse(metrics.get("." + STORE_NAME + "--multiget_retry_request_success_key_count.Rate").value() > 0);
assertFalse(batchGetRequestContext.numberOfKeysCompletedInRetryRequest.get() > 0);
testBatchGetAndvalidateMetrics(true, true, false);
}
}
}

0 comments on commit dbd8ccf

Please sign in to comment.