From 8a86b8bf23ed2d969d54ae834593c60139b4cd57 Mon Sep 17 00:00:00 2001 From: Yingyi Zhang Date: Wed, 15 May 2019 16:25:08 -0700 Subject: [PATCH 1/8] Exclude timed out response from adaptive tracker's histogram 1. Introduce router error code into operation tracker's onResponse(). 2. Update histogram when error code doesn't equal OperationTimedOut. This will help adaptive tracker keep valid latency data points in histogram and will issue 2nd request in time if server crashed or is down for deployment. --- .../NetworkClient.java | 4 +- .../AdaptiveOperationTracker.java | 10 ++- .../DeleteOperation.java | 6 +- .../GetBlobInfoOperation.java | 65 +++++++++-------- .../GetBlobOperation.java | 67 ++++++++++------- .../OperationTracker.java | 7 +- .../com.github.ambry.router/PutOperation.java | 4 +- .../SimpleOperationTracker.java | 2 +- .../TtlUpdateOperation.java | 6 +- .../AdaptiveOperationTrackerTest.java | 10 +-- .../GetBlobInfoOperationTest.java | 73 +++++++++++++++++-- .../GetBlobOperationTest.java | 65 ++++++++++++++++- .../OperationTrackerTest.java | 50 ++++++------- .../RouterTestHelpers.java | 26 ++++++- 14 files changed, 284 insertions(+), 111 deletions(-) diff --git a/ambry-network/src/main/java/com.github.ambry.network/NetworkClient.java b/ambry-network/src/main/java/com.github.ambry.network/NetworkClient.java index 9e86e5f549..d14e59de9c 100644 --- a/ambry-network/src/main/java/com.github.ambry.network/NetworkClient.java +++ b/ambry-network/src/main/java/com.github.ambry.network/NetworkClient.java @@ -228,7 +228,7 @@ public int warmUpConnections(List dataNodeIds, int connectionWarmUpP connId); expectedConnections++; } catch (IOException e) { - logger.error("Received exception while warming up connection: {}", e); + logger.error("Received exception while warming up connection: ", e); } } } @@ -242,7 +242,7 @@ public int warmUpConnections(List dataNodeIds, int connectionWarmUpP failedConnections += selector.disconnected().size(); handleSelectorEvents(null); } catch (IOException e) { - logger.error("Warm up received unexpected error while polling: {}", e); + logger.error("Warm up received unexpected error while polling: ", e); } if (System.currentTimeMillis() - startTime > timeForWarmUp) { break; diff --git a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java index e8e8b07fe7..47ac85f04d 100644 --- a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java +++ b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java @@ -77,15 +77,17 @@ class AdaptiveOperationTracker extends SimpleOperationTracker { } @Override - public void onResponse(ReplicaId replicaId, boolean isSuccessFul) { - super.onResponse(replicaId, isSuccessFul); + public void onResponse(ReplicaId replicaId, boolean isSuccessFul, RouterErrorCode routerErrorCode) { + super.onResponse(replicaId, isSuccessFul, routerErrorCode); long elapsedTime; if (unexpiredRequestSendTimes.containsKey(replicaId)) { elapsedTime = time.milliseconds() - unexpiredRequestSendTimes.remove(replicaId).getSecond(); } else { elapsedTime = time.milliseconds() - expiredRequestSendTimes.remove(replicaId); } - getLatencyHistogram(replicaId).update(elapsedTime); + if (routerErrorCode != RouterErrorCode.OperationTimedOut) { + getLatencyHistogram(replicaId).update(elapsedTime); + } } @Override @@ -101,7 +103,7 @@ public Iterator getReplicaIterator() { * @return the {@link Histogram} that tracks requests to the class of replicas (intra or inter DC) that * {@code replicaId} belongs to. */ - private Histogram getLatencyHistogram(ReplicaId replicaId) { + Histogram getLatencyHistogram(ReplicaId replicaId) { if (replicaId.getDataNodeId().getDatacenterName().equals(datacenterName)) { return localColoTracker; } diff --git a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java index c7eb17fc58..c2e4bd09bb 100644 --- a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java +++ b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java @@ -252,7 +252,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo switch (serverErrorCode) { case No_Error: case Blob_Deleted: - operationTracker.onResponse(replica, true); + operationTracker.onResponse(replica, true, null); if (RouterUtils.isRemoteReplica(routerConfig, replica)) { logger.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(), replica.getDataNodeId().getDatacenterName()); @@ -303,7 +303,7 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) { resolvedRouterErrorCode = error; } } - operationTracker.onResponse(replica, false); + operationTracker.onResponse(replica, false, resolvedRouterErrorCode); if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) { routerMetrics.routerRequestErrorCount.inc(); } @@ -315,7 +315,7 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) { */ private void checkAndMaybeComplete() { // operationCompleted is true if Blob_Authorization_Failure was received. - if (operationTracker.isDone() || operationCompleted == true) { + if (operationTracker.isDone() || operationCompleted) { if (!operationTracker.hasSucceeded()) { setOperationException( new RouterException("The DeleteOperation could not be completed.", resolvedRouterErrorCode)); diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java index 9856f66c2b..67d35df9f2 100644 --- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java +++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java @@ -136,7 +136,7 @@ private void cleanupExpiredInFlightRequests() { while (inFlightRequestsIterator.hasNext()) { Map.Entry entry = inFlightRequestsIterator.next(); if (time.milliseconds() - entry.getValue().startTimeMs > routerConfig.routerRequestTimeoutMs) { - onErrorResponse(entry.getValue().replicaId); + onErrorResponse(entry.getValue().replicaId, RouterErrorCode.OperationTimedOut); logger.trace("GetBlobInfoRequest with correlationId {} in flight has expired for replica {} ", entry.getKey(), entry.getValue().replicaId.getDataNodeId()); // Do not notify this as a failure to the response handler, as this timeout could simply be due to @@ -208,7 +208,7 @@ void handleResponse(ResponseInfo responseInfo, GetResponse getResponse) { logger.trace("GetBlobInfoRequest with response correlationId {} timed out for replica {} ", correlationId, getRequestInfo.replicaId.getDataNodeId()); setOperationException(new RouterException("Operation timed out", RouterErrorCode.OperationTimedOut)); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.OperationTimedOut); } else { if (getResponse == null) { logger.trace( @@ -216,7 +216,7 @@ void handleResponse(ResponseInfo responseInfo, GetResponse getResponse) { correlationId, getRequestInfo.replicaId.getDataNodeId()); setOperationException(new RouterException("Response deserialization received an unexpected error", RouterErrorCode.UnexpectedInternalError)); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.UnexpectedInternalError); } else { if (getResponse.getCorrelationId() != correlationId) { // The NetworkClient associates a response with a request based on the fact that only one request is sent @@ -230,7 +230,7 @@ void handleResponse(ResponseInfo responseInfo, GetResponse getResponse) { "The correlation id in the GetResponse " + getResponse.getCorrelationId() + "is not the same as the correlation id in the associated GetRequest: " + correlationId, RouterErrorCode.UnexpectedInternalError)); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.UnexpectedInternalError); // we do not notify the ResponseHandler responsible for failure detection as this is an unexpected error. } else { try { @@ -244,7 +244,7 @@ void handleResponse(ResponseInfo responseInfo, GetResponse getResponse) { routerMetrics.responseDeserializationErrorCount.inc(); setOperationException(new RouterException("Response deserialization received an unexpected error", e, RouterErrorCode.UnexpectedInternalError)); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.UnexpectedInternalError); } } } @@ -269,7 +269,7 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon setOperationException(new RouterException( "Unexpected number of partition responses, expected: 1, " + "received: " + partitionsInResponse, RouterErrorCode.UnexpectedInternalError)); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.UnexpectedInternalError); // Again, no need to notify the responseHandler. } else { getError = getResponse.getPartitionResponseInfoList().get(0).getErrorCode(); @@ -280,12 +280,12 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon setOperationException(new RouterException( "Unexpected number of messages in a partition response, expected: 1, " + "received: " + msgsInResponse, RouterErrorCode.UnexpectedInternalError)); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.UnexpectedInternalError); } else { MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0); MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0); handleBody(getResponse.getInputStream(), messageMetadata, messageInfo); - operationTracker.onResponse(getRequestInfo.replicaId, true); + operationTracker.onResponse(getRequestInfo.replicaId, true, null); if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) { logger.trace("Cross colo request successful for remote replica in {} ", getRequestInfo.replicaId.getDataNodeId().getDatacenterName()); @@ -294,35 +294,34 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon } } else { // process and set the most relevant exception. - if (getError != ServerErrorCode.No_Error) { - logger.trace("Replica {} returned error {} with response correlationId {} ", - getRequestInfo.replicaId.getDataNodeId(), getError, getResponse.getCorrelationId()); - } - processServerError(getError); + logger.trace("Replica {} returned error {} with response correlationId {} ", + getRequestInfo.replicaId.getDataNodeId(), getError, getResponse.getCorrelationId()); + RouterErrorCode routerErrorCode = processServerError(getError); if (getError == ServerErrorCode.Blob_Deleted || getError == ServerErrorCode.Blob_Expired || getError == ServerErrorCode.Blob_Authorization_Failure) { // this is a successful response and one that completes the operation regardless of whether the // success target has been reached or not. operationCompleted = true; - } else { - onErrorResponse(getRequestInfo.replicaId); } + // any server error code that is not equal to ServerErrorCode.No_Error, the onErrorResponse should be invoked + // because the operation itself doesn't succeed although the response in some cases is successful (i.e. Blob_Deleted) + onErrorResponse(getRequestInfo.replicaId, routerErrorCode); } } } else { logger.trace("Replica {} returned an error {} for a GetBlobInfoRequest with response correlationId : {} ", getRequestInfo.replicaId.getDataNodeId(), getError, getResponse.getCorrelationId()); - processServerError(getError); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, processServerError(getError)); } } /** * Perform the necessary actions when a request to a replica fails. * @param replicaId the {@link ReplicaId} associated with the failed response. + * @param routerErrorCode the {@link RouterErrorCode} associated with the failed response. */ - void onErrorResponse(ReplicaId replicaId) { - operationTracker.onResponse(replicaId, false); + void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) { + operationTracker.onResponse(replicaId, false, routerErrorCode); routerMetrics.routerRequestErrorCount.inc(); routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getBlobInfoRequestErrorCount.inc(); } @@ -382,36 +381,37 @@ private void handleBody(InputStream payload, MessageMetadata messageMetadata, Me /** * Process the given {@link ServerErrorCode} and set operation status accordingly. * @param errorCode the {@link ServerErrorCode} to process. + * @return the {@link RouterErrorCode} mapped from server error code. */ - private void processServerError(ServerErrorCode errorCode) { + private RouterErrorCode processServerError(ServerErrorCode errorCode) { + RouterErrorCode resolvedRouterErrorCode; switch (errorCode) { case Blob_Authorization_Failure: logger.trace("Requested blob authorization failed"); - setOperationException( - new RouterException("Server returned: " + errorCode, RouterErrorCode.BlobAuthorizationFailure)); + resolvedRouterErrorCode = RouterErrorCode.BlobAuthorizationFailure; break; case Blob_Deleted: logger.trace("Requested blob was deleted"); - setOperationException(new RouterException("Server returned: " + errorCode, RouterErrorCode.BlobDeleted)); + resolvedRouterErrorCode = RouterErrorCode.BlobDeleted; break; case Blob_Expired: logger.trace("Requested blob has expired"); - setOperationException(new RouterException("Server returned: " + errorCode, RouterErrorCode.BlobExpired)); + resolvedRouterErrorCode = RouterErrorCode.BlobExpired; break; case Blob_Not_Found: logger.trace("Requested blob was not found on this server"); - setOperationException(new RouterException("Server returned: " + errorCode, RouterErrorCode.BlobDoesNotExist)); + resolvedRouterErrorCode = RouterErrorCode.BlobDoesNotExist; break; case Disk_Unavailable: case Replica_Unavailable: logger.trace("Disk or replica on which the requested blob resides is not accessible"); - setOperationException(new RouterException("Server returned: " + errorCode, RouterErrorCode.AmbryUnavailable)); + resolvedRouterErrorCode = RouterErrorCode.AmbryUnavailable; break; default: - setOperationException( - new RouterException("Server returned: " + errorCode, RouterErrorCode.UnexpectedInternalError)); - break; + resolvedRouterErrorCode = RouterErrorCode.UnexpectedInternalError; } + setOperationException(new RouterException("Server returned: " + errorCode, resolvedRouterErrorCode)); + return resolvedRouterErrorCode; } /** @@ -444,5 +444,12 @@ private void checkAndMaybeComplete() { NonBlockingRouter.completeOperation(null, getOperationCallback, operationResult, e); } } + + /** + * @return {@link OperationTracker} associated with this operation + */ + OperationTracker getOperationTrackerInUse() { + return operationTracker; + } } diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java index 8932c7d907..d65908e253 100644 --- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java +++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java @@ -300,6 +300,10 @@ void poll(RequestRegistrationCallback requestRegistrationCallback) } } + OperationTracker getFirstChunkOperationTrackerInUse() { + return firstChunk.getChunkOperationTrackerInUse(); + } + // ReadableStreamChannel implementation: /** @@ -629,7 +633,7 @@ private void cleanupExpiredInFlightRequests() { while (inFlightRequestsIterator.hasNext()) { Map.Entry entry = inFlightRequestsIterator.next(); if (time.milliseconds() - entry.getValue().startTimeMs > routerConfig.routerRequestTimeoutMs) { - onErrorResponse(entry.getValue().replicaId); + onErrorResponse(entry.getValue().replicaId, RouterErrorCode.OperationTimedOut); logger.trace("GetBlobRequest with correlationId {} in flight has expired for replica {} ", entry.getKey(), entry.getValue().replicaId.getDataNodeId()); // Do not notify this as a failure to the response handler, as this timeout could simply be due to @@ -738,10 +742,13 @@ void handleResponse(ResponseInfo responseInfo, GetResponse getResponse) { routerMetrics.getDataNodeBasedMetrics(getRequestInfo.replicaId.getDataNodeId()).getRequestLatencyMs.update( requestLatencyMs); if (responseInfo.getError() != null) { + // responseInfo.getError() returns NetworkClientErrorCode. If error is not null, it probably means (1) connection + // checkout timed out; (2) pending connection timed out; (3) established connection timed out. In all these cases, + // the latency histogram in adaptive operation tracker should not be updated. logger.trace("GetBlobRequest with response correlationId {} timed out for replica {} ", correlationId, getRequestInfo.replicaId.getDataNodeId()); chunkException = new RouterException("Operation timed out", RouterErrorCode.OperationTimedOut); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.OperationTimedOut); } else { if (getResponse == null) { logger.trace( @@ -749,7 +756,7 @@ void handleResponse(ResponseInfo responseInfo, GetResponse getResponse) { correlationId, getRequestInfo.replicaId.getDataNodeId()); chunkException = new RouterException("Response deserialization received an unexpected error", RouterErrorCode.UnexpectedInternalError); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.UnexpectedInternalError); } else { if (getResponse.getCorrelationId() != correlationId) { // The NetworkClient associates a response with a request based on the fact that only one request is sent @@ -763,7 +770,7 @@ void handleResponse(ResponseInfo responseInfo, GetResponse getResponse) { "The correlation id in the GetResponse " + getResponse.getCorrelationId() + " is not the same as the correlation id in the associated GetRequest: " + correlationId, RouterErrorCode.UnexpectedInternalError); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.UnexpectedInternalError); // we do not notify the ResponseHandler responsible for failure detection as this is an unexpected error. } else { try { @@ -777,7 +784,7 @@ void handleResponse(ResponseInfo responseInfo, GetResponse getResponse) { routerMetrics.responseDeserializationErrorCount.inc(); chunkException = new RouterException("Response deserialization received an unexpected error", e, RouterErrorCode.UnexpectedInternalError); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, RouterErrorCode.UnexpectedInternalError); } } } @@ -860,7 +867,7 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0); MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0); handleBody(getResponse.getInputStream(), messageMetadata, messageInfo); - chunkOperationTracker.onResponse(getRequestInfo.replicaId, true); + chunkOperationTracker.onResponse(getRequestInfo.replicaId, true, null); if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) { logger.trace("Cross colo request successful for remote replica in {} ", getRequestInfo.replicaId.getDataNodeId().getDatacenterName()); @@ -869,32 +876,33 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g } } else { // process and set the most relevant exception. - processServerError(getError); + RouterErrorCode routerErrorCode = processServerError(getError); if (getError == ServerErrorCode.Blob_Deleted || getError == ServerErrorCode.Blob_Expired || getError == ServerErrorCode.Blob_Authorization_Failure) { // this is a successful response and one that completes the operation regardless of whether the // success target has been reached or not. chunkCompleted = true; - } else { - onErrorResponse(getRequestInfo.replicaId); } + // any server error code that is not equal to ServerErrorCode.No_Error, the onErrorResponse should be invoked + // because the operation itself doesn't succeed although the response in some cases is successful (i.e. Blob_Deleted) + onErrorResponse(getRequestInfo.replicaId, routerErrorCode); } } } else { logger.trace("Replica {} returned an error {} for a GetBlobRequest with response correlationId : {} ", getRequestInfo.replicaId.getDataNodeId(), getError, getResponse.getCorrelationId()); // process and set the most relevant exception. - processServerError(getError); - onErrorResponse(getRequestInfo.replicaId); + onErrorResponse(getRequestInfo.replicaId, processServerError(getError)); } } /** * Perform the necessary actions when a request to a replica fails. * @param replicaId the {@link ReplicaId} associated with the failed response. + * @param routerErrorCode the {@link RouterErrorCode} associated with the failed response. */ - void onErrorResponse(ReplicaId replicaId) { - chunkOperationTracker.onResponse(replicaId, false); + void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) { + chunkOperationTracker.onResponse(replicaId, false, routerErrorCode); routerMetrics.routerRequestErrorCount.inc(); routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getRequestErrorCount.inc(); } @@ -904,9 +912,12 @@ void onErrorResponse(ReplicaId replicaId) { * Receiving a {@link ServerErrorCode#Blob_Deleted}, {@link ServerErrorCode#Blob_Expired} or * {@link ServerErrorCode#Blob_Not_Found} is unexpected for all chunks except for the first. * @param errorCode the {@link ServerErrorCode} to process. + * @return the {@link RouterErrorCode} mapped from input server error code. */ - void processServerError(ServerErrorCode errorCode) { - setChunkException(new RouterException("Server returned: " + errorCode, RouterErrorCode.UnexpectedInternalError)); + RouterErrorCode processServerError(ServerErrorCode errorCode) { + RouterErrorCode resolvedRouterErrorCode = RouterErrorCode.UnexpectedInternalError; + setChunkException(new RouterException("Server returned: " + errorCode, resolvedRouterErrorCode)); + return resolvedRouterErrorCode; } /** @@ -973,6 +984,10 @@ boolean isInProgress() { boolean isComplete() { return state == ChunkState.Complete; } + + OperationTracker getChunkOperationTrackerInUse() { + return chunkOperationTracker; + } } /** @@ -1044,7 +1059,7 @@ protected void maybeProcessCallbacks() { logger.trace("BlobContent available to process for Metadata blob {}", blobId); } else { logger.trace("Processing stored decryption callback result for simple blob {}", blobId); - // Incase of simple blobs, user-metadata may or may not be passed into decryption job based on GetOptions flag. + // In case of simple blobs, user-metadata may or may not be passed into decryption job based on GetOptions flag. // Only in-case of GetBlobInfo and GetBlobAll, user-metadata is required to be decrypted if (decryptCallbackResultInfo.result.getDecryptedUserMetadata() != null) { blobInfo = new BlobInfo(serverBlobProperties, @@ -1148,31 +1163,31 @@ void handleBody(InputStream payload, MessageMetadata messageMetadata, MessageInf * {@link ServerErrorCode#Blob_Not_Found} is not unexpected for the first chunk, unlike for subsequent chunks. */ @Override - void processServerError(ServerErrorCode errorCode) { + RouterErrorCode processServerError(ServerErrorCode errorCode) { logger.trace("Server returned an error: {} ", errorCode); + RouterErrorCode resolvedRouterErrorCode; switch (errorCode) { case Blob_Authorization_Failure: - setChunkException( - new RouterException("Server returned: " + errorCode, RouterErrorCode.BlobAuthorizationFailure)); + resolvedRouterErrorCode = RouterErrorCode.BlobAuthorizationFailure; break; case Blob_Deleted: - setChunkException(new RouterException("Server returned: " + errorCode, RouterErrorCode.BlobDeleted)); + resolvedRouterErrorCode = RouterErrorCode.BlobDeleted; break; case Blob_Expired: - setChunkException(new RouterException("Server returned: " + errorCode, RouterErrorCode.BlobExpired)); + resolvedRouterErrorCode = RouterErrorCode.BlobExpired; break; case Blob_Not_Found: - setChunkException(new RouterException("Server returned: " + errorCode, RouterErrorCode.BlobDoesNotExist)); + resolvedRouterErrorCode = RouterErrorCode.BlobDoesNotExist; break; case Disk_Unavailable: case Replica_Unavailable: - setChunkException(new RouterException("Server returned: " + errorCode, RouterErrorCode.AmbryUnavailable)); + resolvedRouterErrorCode = RouterErrorCode.AmbryUnavailable; break; default: - setChunkException( - new RouterException("Server returned: " + errorCode, RouterErrorCode.UnexpectedInternalError)); - break; + resolvedRouterErrorCode = RouterErrorCode.UnexpectedInternalError; } + setChunkException(new RouterException("Server returned: " + errorCode, resolvedRouterErrorCode)); + return resolvedRouterErrorCode; } /** diff --git a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java index e8a1a81747..a1b040f70b 100644 --- a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java +++ b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java @@ -26,7 +26,7 @@ * next replica to send a request. * * When an operation is progressing by receiving responses from replicas, its {@code OperationTracker} - * needs to be informed by calling {@link #onResponse(ReplicaId, boolean)}. + * needs to be informed by calling {@link #onResponse(ReplicaId, boolean, RouterErrorCode)}. * * Typical usage of an {@code OperationTracker} would be: * 
@@ -63,11 +63,12 @@ interface OperationTracker {
   /**
    * Accounts for successful or failed response from a replica. Must invoke this method
    * if a successful or failed response is received for a replica.
-   *
    * @param replicaId ReplicaId associated with this response.
    * @param isSuccessful Whether the request to the replicaId is successful or not.
+   * @param routerErrorCode The {@link RouterErrorCode} associated with this request if it failed. This can be null if
+   *                        request is successful.
    */
-  void onResponse(ReplicaId replicaId, boolean isSuccessful);
+  void onResponse(ReplicaId replicaId, boolean isSuccessful, RouterErrorCode routerErrorCode);
 
   /**
    * Provide an iterator to the replicas to which requests may be sent. Each time when start to iterate
diff --git a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
index e613379e48..56684572b4 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
@@ -1407,7 +1407,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
         }
       }
       if (isSuccessful) {
-        operationTracker.onResponse(chunkPutRequestInfo.replicaId, true);
+        operationTracker.onResponse(chunkPutRequestInfo.replicaId, true, null);
         if (RouterUtils.isRemoteReplica(routerConfig, chunkPutRequestInfo.replicaId)) {
           logger.trace("Cross colo request successful for remote replica in {} ",
               chunkPutRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -1424,7 +1424,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
      * @param replicaId the {@link ReplicaId} associated with the failed response.
      */
     void onErrorResponse(ReplicaId replicaId) {
-      operationTracker.onResponse(replicaId, false);
+      operationTracker.onResponse(replicaId, false, RouterErrorCode.AmbryUnavailable);
       routerMetrics.routerRequestErrorCount.inc();
       routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).putRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
index 9bbc94a5b9..98743edf77 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
@@ -185,7 +185,7 @@ public boolean isDone() {
   }
 
   @Override
-  public void onResponse(ReplicaId replicaId, boolean isSuccessFul) {
+  public void onResponse(ReplicaId replicaId, boolean isSuccessFul, RouterErrorCode routerErrorCode) {
     inflightCount--;
     if (isSuccessFul) {
       succeededCount++;
diff --git a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
index 1b3fa1144b..a65974f580 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
@@ -243,7 +243,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo
     switch (serverErrorCode) {
       case No_Error:
       case Blob_Already_Updated:
-        operationTracker.onResponse(replica, true);
+        operationTracker.onResponse(replica, true, null);
         if (RouterUtils.isRemoteReplica(routerConfig, replica)) {
           LOGGER.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(),
               replica.getDataNodeId().getDatacenterName());
@@ -297,7 +297,7 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
         resolvedRouterErrorCode = error;
       }
     }
-    operationTracker.onResponse(replica, false);
+    operationTracker.onResponse(replica, false, resolvedRouterErrorCode);
     if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) {
       routerMetrics.routerRequestErrorCount.inc();
     }
@@ -309,7 +309,7 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
    */
   private void checkAndMaybeComplete() {
     // operationCompleted is true if Blob_Authorization_Failure was received.
-    if (operationTracker.isDone() || operationCompleted == true) {
+    if (operationTracker.isDone() || operationCompleted) {
       if (!operationTracker.hasSucceeded()) {
         setOperationException(
             new RouterException("The TtlUpdateOperation could not be completed.", resolvedRouterErrorCode));
diff --git a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
index 03125132d2..0435f9aea0 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
@@ -144,7 +144,7 @@ public void adaptationTest() throws InterruptedException {
     // generate a response for every request and make sure there are no errors
     for (int i = 0; i < REPLICA_COUNT; i++) {
       assertFalse("Operation should not be done", ot.isDone());
-      ot.onResponse(inflightReplicas.poll(), true);
+      ot.onResponse(inflightReplicas.poll(), true, null);
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be REPLICA_COUNT - 2
@@ -191,13 +191,13 @@ public void noUnexpiredRequestsTest() throws InterruptedException {
     sendRequests(ot, 1);
     // 1-2-0-0
     // provide a response to the second request that is not a success
-    ot.onResponse(inflightReplicas.pollLast(), false);
+    ot.onResponse(inflightReplicas.pollLast(), false, null);
     // 1-1-0-1
     assertFalse("Operation should not be done", ot.isDone());
     // should now be able to send one more request
     sendRequests(ot, 1);
     // 0-2-0-1
-    ot.onResponse(inflightReplicas.pollLast(), true);
+    ot.onResponse(inflightReplicas.pollLast(), true, null);
     // 0-1-1-1
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -235,7 +235,7 @@ public void trackerUpdateBetweenHasNextAndNextTest() throws InterruptedException
 
     sendRequests(ot, 1);
     // 1-2-0-0
-    ot.onResponse(inflightReplicas.pollLast(), true);
+    ot.onResponse(inflightReplicas.pollLast(), true, null);
     // 1-1-1-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -392,7 +392,7 @@ private void verifyHistogramRecording(OperationTracker ot, boolean succeedReques
       Double[] expectedAverages, Histogram tracker) throws InterruptedException {
     for (double expectedAverage : expectedAverages) {
       time.sleep(timeIncrement);
-      ot.onResponse(inflightReplicas.poll(), succeedRequests);
+      ot.onResponse(inflightReplicas.poll(), succeedRequests, null);
       assertEquals("Average does not match. Histogram recording may be incorrect", expectedAverage,
           tracker.getSnapshot().getMean(), 0.001);
     }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java b/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
index 359991778c..76e6e1f4b9 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
@@ -16,6 +16,7 @@
 import com.github.ambry.account.InMemAccountService;
 import com.github.ambry.clustermap.ClusterMapUtils;
 import com.github.ambry.clustermap.MockClusterMap;
+import com.github.ambry.clustermap.ReplicaId;
 import com.github.ambry.commons.BlobId;
 import com.github.ambry.commons.ByteBufferReadableStreamChannel;
 import com.github.ambry.commons.LoggingNotificationSystem;
@@ -60,6 +61,7 @@
 import org.junit.runners.Parameterized;
 
 import static com.github.ambry.router.PutManagerTest.*;
+import static org.junit.Assume.*;
 
 
 /**
@@ -102,6 +104,9 @@ public class GetBlobInfoOperationTest {
   private MockKeyManagementService kms = null;
   private MockCryptoService cryptoService = null;
   private CryptoJobHandler cryptoJobHandler = null;
+  private ReplicaId localReplica;
+  private ReplicaId remoteReplica;
+  private String localDcName;
 
   private class GetTestRequestRegistrationCallbackImpl implements RequestRegistrationCallback {
     private List requestListToFill;
@@ -135,6 +140,7 @@ public GetBlobInfoOperationTest(String operationTrackerType, boolean testEncrypt
     VerifiableProperties vprops = new VerifiableProperties(getNonBlockingRouterProperties());
     routerConfig = new RouterConfig(vprops);
     mockClusterMap = new MockClusterMap();
+    localDcName = mockClusterMap.getDatacenterName(mockClusterMap.getLocalDatacenterId());
     routerMetrics = new NonBlockingRouterMetrics(mockClusterMap, routerConfig);
     options = new GetBlobOptionsInternal(
         new GetBlobOptionsBuilder().operationType(GetBlobOptions.OperationType.BlobInfo).build(), false,
@@ -165,9 +171,11 @@ networkClientFactory, new LoggingNotificationSystem(), mockClusterMap, kms, cryp
     String blobIdStr =
         router.putBlob(blobProperties, userMetadata, putChannel, new PutBlobOptionsBuilder().build()).get();
     blobId = RouterUtils.getBlobIdFromString(blobIdStr, mockClusterMap);
+    localReplica = RouterTestHelpers.getAnyReplica(blobId, true, localDcName);
+    remoteReplica = RouterTestHelpers.getAnyReplica(blobId, false, localDcName);
     networkClient = networkClientFactory.getNetworkClient();
     router.close();
-    routerCallback = new MockRouterCallback(networkClient, Collections.EMPTY_LIST);
+    routerCallback = new MockRouterCallback(networkClient, Collections.emptyList());
     if (testEncryption) {
       instantiateCryptoComponents(vprops);
     }
@@ -269,20 +277,23 @@ public void testPollAndResponseHandling() throws Exception {
 
   /**
    * Test the case where all requests time out within the GetOperation.
-   * @throws Exception
    */
   @Test
-  public void testRouterRequestTimeoutAllFailure() throws Exception {
+  public void testRouterRequestTimeoutAllFailure() {
     NonBlockingRouter.currentOperationsCount.incrementAndGet();
     GetBlobInfoOperation op =
         new GetBlobInfoOperation(routerConfig, routerMetrics, mockClusterMap, responseHandler, blobId, options, null,
             routerCallback, kms, cryptoService, cryptoJobHandler, time, false);
     requestRegistrationCallback.requestListToFill = new ArrayList<>();
     op.poll(requestRegistrationCallback);
+    int count = 0;
     while (!op.isOperationComplete()) {
       time.sleep(routerConfig.routerRequestTimeoutMs + 1);
       op.poll(requestRegistrationCallback);
+      ++count;
+      System.out.println("counter = " + count);
     }
+    System.out.println("replicaCnt = " + replicasCount);
     // At this time requests would have been created for all replicas, as none of them were delivered,
     // and cross-colo proxying is enabled by default.
     Assert.assertEquals("Must have attempted sending requests to all replicas", replicasCount,
@@ -290,6 +301,58 @@ public void testRouterRequestTimeoutAllFailure() throws Exception {
     Assert.assertTrue("Operation should be complete at this time", op.isOperationComplete());
     RouterException routerException = (RouterException) op.getOperationException();
     Assert.assertEquals(RouterErrorCode.OperationTimedOut, routerException.getErrorCode());
+    // test that time out response shouldn't update the Histogram
+    assumeTrue(operationTrackerType.equals(AdaptiveOperationTracker.class.getSimpleName()));
+    AdaptiveOperationTracker tracker = (AdaptiveOperationTracker) op.getOperationTrackerInUse();
+    Assert.assertEquals("Timed-out response shouldn't be counted into local colo latency histogram", 0,
+        tracker.getLatencyHistogram(localReplica).getCount());
+    Assert.assertEquals("Timed-out response shouldn't be counted into cross colo latency histogram", 0,
+        tracker.getLatencyHistogram(remoteReplica).getCount());
+  }
+
+  /**
+   * Test the case where all local replicas and one remote replica timed out. The rest remote replicas respond
+   * with Blob_Not_Found.
+   * @throws Exception
+   */
+  @Test
+  public void testTimeoutAndBlobNotFound() throws Exception {
+    assumeTrue(operationTrackerType.equals(AdaptiveOperationTracker.class.getSimpleName()));
+    correlationIdToGetOperation.clear();
+    for (MockServer server : mockServerLayout.getMockServers()) {
+      if (!server.getDataCenter().equals(localDcName)) {
+        server.setServerErrorForAllRequests(ServerErrorCode.Blob_Not_Found);
+      }
+    }
+    NonBlockingRouter.currentOperationsCount.incrementAndGet();
+    GetBlobInfoOperation op =
+        new GetBlobInfoOperation(routerConfig, routerMetrics, mockClusterMap, responseHandler, blobId, options, null,
+            routerCallback, kms, cryptoService, cryptoJobHandler, time, false);
+    requestRegistrationCallback.requestListToFill = new ArrayList<>();
+    AdaptiveOperationTracker tracker = (AdaptiveOperationTracker) op.getOperationTrackerInUse();
+
+    op.poll(requestRegistrationCallback);
+    Assert.assertEquals("There should only be as many requests at this point as requestParallelism", requestParallelism,
+        correlationIdToGetOperation.size());
+    int count = 0;
+    while (!op.isOperationComplete()) {
+      time.sleep(routerConfig.routerRequestTimeoutMs + 1);
+      op.poll(requestRegistrationCallback);
+      if (++count == 2) {
+        // exit loop to let remote replicas complete the response.
+        break;
+      }
+    }
+    // 2 + 2 requests have been sent out and all of them timed out. Nest, complete operation on remaining replicas
+    completeOp(op);
+    RouterException routerException = (RouterException) op.getOperationException();
+    // error code should be OperationTimedOut because it precedes BlobDoesNotExist
+    Assert.assertEquals(RouterErrorCode.OperationTimedOut, routerException.getErrorCode());
+    Assert.assertEquals("The number of data points in local colo latency histogram is not expected", 0,
+        tracker.getLatencyHistogram(localReplica).getCount());
+    // the count of data points in Histogram should be 5 because 9(total replicas) - 4(# of timed out request) = 5
+    Assert.assertEquals("The number of data points in cross colo latency histogram is not expected", 5,
+        tracker.getLatencyHistogram(remoteReplica).getCount());
   }
 
   /**
@@ -568,7 +631,7 @@ private List sendAndWaitForResponses(List requestList
    * @param op the {@link GetBlobInfoOperation} that should have completed.
    */
   private void assertSuccess(GetBlobInfoOperation op) {
-    Assert.assertEquals("Null expected", null, op.getOperationException());
+    Assert.assertNull("Null expected", op.getOperationException());
     BlobInfo blobInfo = op.getOperationResult().getBlobResult.getBlobInfo();
     Assert.assertNull("Unexpected blob data channel in operation result",
         op.getOperationResult().getBlobResult.getBlobDataChannel());
@@ -586,7 +649,7 @@ private void assertSuccess(GetBlobInfoOperation op) {
   private Properties getNonBlockingRouterProperties() {
     Properties properties = new Properties();
     properties.setProperty("router.hostname", "localhost");
-    properties.setProperty("router.datacenter.name", "DC1");
+    properties.setProperty("router.datacenter.name", "DC3");
     properties.setProperty("router.get.request.parallelism", Integer.toString(requestParallelism));
     properties.setProperty("router.get.success.target", Integer.toString(successTarget));
     properties.setProperty("router.get.operation.tracker.type", operationTrackerType);
diff --git a/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java b/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
index b1184946cd..ab5b04c129 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
@@ -14,6 +14,7 @@
 package com.github.ambry.router;
 
 import com.codahale.metrics.Counter;
+import com.codahale.metrics.Histogram;
 import com.github.ambry.account.InMemAccountService;
 import com.github.ambry.clustermap.MockClusterMap;
 import com.github.ambry.clustermap.PartitionId;
@@ -82,6 +83,7 @@
 
 import static com.github.ambry.router.PutManagerTest.*;
 import static com.github.ambry.router.RouterTestHelpers.*;
+import static org.junit.Assume.*;
 
 
 /**
@@ -119,6 +121,7 @@ public class GetBlobOperationTest {
   private MockKeyManagementService kms = null;
   private MockCryptoService cryptoService = null;
   private CryptoJobHandler cryptoJobHandler = null;
+  private String localDcName;
 
   // Certain tests recreate the routerConfig with different properties.
   private RouterConfig routerConfig;
@@ -200,6 +203,8 @@ public GetBlobOperationTest(String operationTrackerType, boolean testEncryption)
     VerifiableProperties vprops = new VerifiableProperties(getDefaultNonBlockingRouterProperties());
     routerConfig = new RouterConfig(vprops);
     mockClusterMap = new MockClusterMap();
+    localDcName = mockClusterMap.getDatacenterName(mockClusterMap.getLocalDatacenterId());
+    System.out.println("clusterm local datacenter id: " + mockClusterMap.getLocalDatacenterId());
     blobIdFactory = new BlobIdFactory(mockClusterMap);
     routerMetrics = new NonBlockingRouterMetrics(mockClusterMap, routerConfig);
     options = new GetBlobOptionsInternal(new GetBlobOptionsBuilder().build(), false, routerMetrics.ageAtGet);
@@ -239,6 +244,7 @@ private void doPut() throws Exception {
     ReadableStreamChannel putChannel = new ByteBufferReadableStreamChannel(ByteBuffer.wrap(putContent));
     blobIdStr = router.putBlob(blobProperties, userMetadata, putChannel, new PutBlobOptionsBuilder().build()).get();
     blobId = RouterUtils.getBlobIdFromString(blobIdStr, mockClusterMap);
+    System.out.println("blob datacenter id: " + blobId.getDatacenterId());
   }
 
   /**
@@ -468,6 +474,63 @@ public void testRouterRequestTimeoutAllFailure() throws Exception {
     Assert.assertEquals("Must have attempted sending requests to all replicas", replicasCount,
         correlationIdToGetOperation.size());
     assertFailureAndCheckErrorCode(op, RouterErrorCode.OperationTimedOut);
+
+    // test that timed out response won't update latency histogram
+    assumeTrue(operationTrackerType.equals(AdaptiveOperationTracker.class.getSimpleName()));
+    AdaptiveOperationTracker tracker = (AdaptiveOperationTracker) op.getFirstChunkOperationTrackerInUse();
+    Histogram localColoTracker =
+        tracker.getLatencyHistogram(RouterTestHelpers.getAnyReplica(blobId, true, localDcName));
+    Histogram crossColoTracker =
+        tracker.getLatencyHistogram(RouterTestHelpers.getAnyReplica(blobId, false, localDcName));
+    Assert.assertEquals("Timed-out response shouldn't be counted into local colo latency histogram", 0,
+        localColoTracker.getCount());
+    Assert.assertEquals("Timed-out response shouldn't be counted into cross colo latency histogram", 0,
+        crossColoTracker.getCount());
+  }
+
+  /**
+   * Test the case where 2 local replicas timed out. The remaining one local replica and rest remote replicas respond
+   * with Blob_Not_Found.
+   * @throws Exception
+   */
+  @Test
+  public void testRequestTimeoutAndBlobNotFound() throws Exception {
+    assumeTrue(operationTrackerType.equals(AdaptiveOperationTracker.class.getSimpleName()));
+    doPut();
+    GetBlobOperation op = createOperation(null);
+    AdaptiveOperationTracker tracker = (AdaptiveOperationTracker) op.getFirstChunkOperationTrackerInUse();
+    correlationIdToGetOperation.clear();
+    for (MockServer server : mockServerLayout.getMockServers()) {
+      server.setServerErrorForAllRequests(ServerErrorCode.Blob_Not_Found);
+    }
+    op.poll(requestRegistrationCallback);
+    time.sleep(routerConfig.routerRequestTimeoutMs + 1);
+
+    // 2 requests have been sent out and both of them timed out. Nest, complete operation on remaining replicas
+    // The request should have response from one local replica and all remote replicas.
+    while (!op.isOperationComplete()) {
+      op.poll(requestRegistrationCallback);
+      List responses = sendAndWaitForResponses(requestRegistrationCallback.requestListToFill);
+      for (ResponseInfo responseInfo : responses) {
+        GetResponse getResponse = responseInfo.getError() == null ? GetResponse.readFrom(
+            new DataInputStream(new ByteBufferInputStream(responseInfo.getResponse())), mockClusterMap) : null;
+        op.handleResponse(responseInfo, getResponse);
+      }
+    }
+
+    RouterException routerException = (RouterException) op.getOperationException();
+    // error code should be OperationTimedOut because it precedes BlobDoesNotExist
+    Assert.assertEquals(RouterErrorCode.OperationTimedOut, routerException.getErrorCode());
+    Histogram localColoTracker =
+        tracker.getLatencyHistogram(RouterTestHelpers.getAnyReplica(blobId, true, localDcName));
+    Histogram crossColoTracker =
+        tracker.getLatencyHistogram(RouterTestHelpers.getAnyReplica(blobId, false, localDcName));
+    // the count of data points in local colo Histogram should be 1, because first 2 request timed out
+    Assert.assertEquals("The number of data points in local colo latency histogram is not expected", 1,
+        localColoTracker.getCount());
+    // the count of data points in cross colo Histogram should be 6 because all remote replicas respond with proper error code
+    Assert.assertEquals("The number of data points in cross colo latency histogram is not expected", 6,
+        crossColoTracker.getCount());
   }
 
   /**
@@ -1418,7 +1481,7 @@ private List sendAndWaitForResponses(List requestList
   private Properties getDefaultNonBlockingRouterProperties() {
     Properties properties = new Properties();
     properties.setProperty("router.hostname", "localhost");
-    properties.setProperty("router.datacenter.name", "DC1");
+    properties.setProperty("router.datacenter.name", "DC3");
     properties.setProperty("router.put.request.parallelism", Integer.toString(3));
     properties.setProperty("router.put.success.target", Integer.toString(2));
     properties.setProperty("router.max.put.chunk.size.bytes", Integer.toString(maxChunkSize));
diff --git a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
index d862353752..9ee09a5f76 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
@@ -117,13 +117,13 @@ public void localSucceedTest() {
     // 0-3-0-0; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), true);
+      ot.onResponse(inflightReplicas.poll(), true, null);
     }
     // 0-1-2-0; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
 
-    ot.onResponse(inflightReplicas.poll(), false);
+    ot.onResponse(inflightReplicas.poll(), false, null);
     // 0-0-2-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -146,12 +146,12 @@ public void localFailTest() {
     sendRequests(ot, 3, false);
     // 0-3-0-0; 9-0-0-0
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), false);
+      ot.onResponse(inflightReplicas.poll(), false, null);
     }
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
     // 0-1-0-2; 9-0-0-0
-    ot.onResponse(inflightReplicas.poll(), true);
+    ot.onResponse(inflightReplicas.poll(), true, null);
     // 0-0-1-2; 9-0-0-0
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -174,14 +174,14 @@ public void localSucceedWithDifferentParameterTest() {
     sendRequests(ot, 2, false);
     // 1-2-0-0; 9-0-0-0
 
-    ot.onResponse(inflightReplicas.poll(), false);
+    ot.onResponse(inflightReplicas.poll(), false, null);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
 
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
-    ot.onResponse(inflightReplicas.poll(), true);
+    ot.onResponse(inflightReplicas.poll(), true, null);
     // 0-1-1-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -212,35 +212,35 @@ public void remoteReplicaTest() {
     // 1-2-0-0; 9-0-0-0
     ReplicaId id = inflightReplicas.poll();
     assertEquals("First request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, false);
+    ot.onResponse(id, false, null);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     id = inflightReplicas.poll();
     assertEquals("Second request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, false);
+    ot.onResponse(id, false, null);
     id = inflightReplicas.poll();
     assertEquals("Third request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, false);
+    ot.onResponse(id, false, null);
     // 0-0-0-3; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 7-2-0-0
     assertFalse("Operation should not be done", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), false);
+      ot.onResponse(inflightReplicas.poll(), false, null);
     }
     // 0-0-0-3; 7-0-0-2
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 5-2-0-2
-    ot.onResponse(inflightReplicas.poll(), false);
+    ot.onResponse(inflightReplicas.poll(), false, null);
     assertFalse("Operation should not be done", ot.isDone());
     // 0-0-0-3; 5-1-0-3
     sendRequests(ot, 1, false);
     // 0-0-0-3; 4-1-0-3
-    ot.onResponse(inflightReplicas.poll(), true);
+    ot.onResponse(inflightReplicas.poll(), true, null);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -262,7 +262,7 @@ public void fullSuccessTargetTest() {
     while (!ot.hasSucceeded()) {
       sendRequests(ot, 3, false);
       for (int i = 0; i < 3; i++) {
-        ot.onResponse(inflightReplicas.poll(), true);
+        ot.onResponse(inflightReplicas.poll(), true, null);
       }
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -295,14 +295,14 @@ public void useReplicaNotSucceededSendTest() {
     localDcName = datanodes.get(0).getDatacenterName();
     OperationTracker ot = getOperationTracker(true, 1, 2, true, Integer.MAX_VALUE);
     sendRequests(ot, 2, true);
-    ot.onResponse(inflightReplicas.poll(), false);
-    ot.onResponse(inflightReplicas.poll(), false);
+    ot.onResponse(inflightReplicas.poll(), false, null);
+    ot.onResponse(inflightReplicas.poll(), false, null);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, true);
-    ot.onResponse(inflightReplicas.poll(), false);
+    ot.onResponse(inflightReplicas.poll(), false, null);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
-    ot.onResponse(inflightReplicas.poll(), true);
+    ot.onResponse(inflightReplicas.poll(), true, null);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -338,12 +338,12 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, false);
+      ot.onResponse(replica, false, null);
     }
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, false);
+      ot.onResponse(replica, false, null);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
@@ -351,7 +351,7 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, true);
+      ot.onResponse(replica, true, null);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -369,7 +369,7 @@ public void replicasOrderingTestOriginatingIsLocal() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, true);
+      ot.onResponse(replica, true, null);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -388,14 +388,14 @@ public void replicasOrderingTestOriginatingNotLocal() {
     sendRequests(ot, 6, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, false); // fail first 3 requests to local
+      ot.onResponse(replica, false, null); // fail first 3 requests to local
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, true);
+      ot.onResponse(replica, true, null);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -416,14 +416,14 @@ public void replicasOrderTestOriginatingDcOnly() {
     assertEquals("Should have 6 replicas", 6, inflightReplicas.size());
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, false); // fail first 3 requests to local replicas
+      ot.onResponse(replica, false, null); // fail first 3 requests to local replicas
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, true);
+      ot.onResponse(replica, true, null);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
diff --git a/ambry-router/src/test/java/com.github.ambry.router/RouterTestHelpers.java b/ambry-router/src/test/java/com.github.ambry.router/RouterTestHelpers.java
index 2072e4487c..abb2a4a1be 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/RouterTestHelpers.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/RouterTestHelpers.java
@@ -226,8 +226,8 @@ static  void assertFailureAndCheckErrorCode(Future future, TestCallback
    * @throws Exception
    */
   static void assertTtl(Router router, Collection blobIds, long expectedTtlSecs) throws Exception {
-    GetBlobOptions options[] = {new GetBlobOptionsBuilder().build(), new GetBlobOptionsBuilder().operationType(
-        GetBlobOptions.OperationType.BlobInfo).build()};
+    GetBlobOptions options[] = {new GetBlobOptionsBuilder().build(),
+        new GetBlobOptionsBuilder().operationType(GetBlobOptions.OperationType.BlobInfo).build()};
     for (String blobId : blobIds) {
       for (GetBlobOptions option : options) {
         GetBlobResult result = router.getBlob(blobId, option).get(AWAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS);
@@ -315,6 +315,28 @@ private static String getRandomBlobId(ClusterMap clusterMap, BlobId.BlobDataType
         Account.UNKNOWN_ACCOUNT_ID, Container.UNKNOWN_CONTAINER_ID, partitionId, false, blobDataType).getID();
   }
 
+  /**
+   * Get any of the replicas from local DC or remote DC (based on the given parameter).
+   * @param blobId
+   * @param fromLocal
+   * @param localDcName
+   * @return
+   */
+  static ReplicaId getAnyReplica(BlobId blobId, boolean fromLocal, String localDcName) {
+    ReplicaId replicaToReturn = null;
+    for (ReplicaId replicaId : blobId.getPartition().getReplicaIds()) {
+      if (fromLocal && replicaId.getDataNodeId().getDatacenterName().equals(localDcName)) {
+        replicaToReturn = replicaId;
+        break;
+      }
+      if (!fromLocal && !replicaId.getDataNodeId().getDatacenterName().equals(localDcName)) {
+        replicaToReturn = replicaId;
+        break;
+      }
+    }
+    return replicaToReturn;
+  }
+
   /**
    * Implement this interface to provide {@link #testWithErrorCodes} with custom verification logic.
    */

From 36fb4823fc60319f098cb1651c6dfab09dcf24e3 Mon Sep 17 00:00:00 2001
From: Yingyi Zhang 
Date: Thu, 16 May 2019 10:00:03 -0700
Subject: [PATCH 2/8] clean code

---
 .../com.github.ambry.router/GetBlobInfoOperationTest.java | 2 --
 .../com.github.ambry.router/GetBlobOperationTest.java     | 2 --
 .../com.github.ambry.router/OperationTrackerTest.java     | 6 ++++--
 .../java/com.github.ambry.router/RouterTestHelpers.java   | 8 ++++----
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java b/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
index 76e6e1f4b9..449036362a 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
@@ -291,9 +291,7 @@ public void testRouterRequestTimeoutAllFailure() {
       time.sleep(routerConfig.routerRequestTimeoutMs + 1);
       op.poll(requestRegistrationCallback);
       ++count;
-      System.out.println("counter = " + count);
     }
-    System.out.println("replicaCnt = " + replicasCount);
     // At this time requests would have been created for all replicas, as none of them were delivered,
     // and cross-colo proxying is enabled by default.
     Assert.assertEquals("Must have attempted sending requests to all replicas", replicasCount,
diff --git a/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java b/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
index ab5b04c129..ce13bd3e34 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
@@ -204,7 +204,6 @@ public GetBlobOperationTest(String operationTrackerType, boolean testEncryption)
     routerConfig = new RouterConfig(vprops);
     mockClusterMap = new MockClusterMap();
     localDcName = mockClusterMap.getDatacenterName(mockClusterMap.getLocalDatacenterId());
-    System.out.println("clusterm local datacenter id: " + mockClusterMap.getLocalDatacenterId());
     blobIdFactory = new BlobIdFactory(mockClusterMap);
     routerMetrics = new NonBlockingRouterMetrics(mockClusterMap, routerConfig);
     options = new GetBlobOptionsInternal(new GetBlobOptionsBuilder().build(), false, routerMetrics.ageAtGet);
@@ -244,7 +243,6 @@ private void doPut() throws Exception {
     ReadableStreamChannel putChannel = new ByteBufferReadableStreamChannel(ByteBuffer.wrap(putContent));
     blobIdStr = router.putBlob(blobProperties, userMetadata, putChannel, new PutBlobOptionsBuilder().build()).get();
     blobId = RouterUtils.getBlobIdFromString(blobIdStr, mockClusterMap);
-    System.out.println("blob datacenter id: " + blobId.getDatacenterId());
   }
 
   /**
diff --git a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
index 9ee09a5f76..f2bb29d1d7 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
@@ -388,7 +388,8 @@ public void replicasOrderingTestOriginatingNotLocal() {
     sendRequests(ot, 6, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, false, null); // fail first 3 requests to local
+      // fail first 3 requests to local
+      ot.onResponse(replica, false, null);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
@@ -416,7 +417,8 @@ public void replicasOrderTestOriginatingDcOnly() {
     assertEquals("Should have 6 replicas", 6, inflightReplicas.size());
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, false, null); // fail first 3 requests to local replicas
+      // fail first 3 requests to local replicas
+      ot.onResponse(replica, false, null);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
diff --git a/ambry-router/src/test/java/com.github.ambry.router/RouterTestHelpers.java b/ambry-router/src/test/java/com.github.ambry.router/RouterTestHelpers.java
index abb2a4a1be..5aac86888f 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/RouterTestHelpers.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/RouterTestHelpers.java
@@ -317,10 +317,10 @@ private static String getRandomBlobId(ClusterMap clusterMap, BlobId.BlobDataType
 
   /**
    * Get any of the replicas from local DC or remote DC (based on the given parameter).
-   * @param blobId
-   * @param fromLocal
-   * @param localDcName
-   * @return
+   * @param blobId the id of blob that sits on the replica
+   * @param fromLocal whether to get replica from local datacenter
+   * @param localDcName the name of local datacenter
+   * @return any {@link ReplicaId} that satisfies requirement
    */
   static ReplicaId getAnyReplica(BlobId blobId, boolean fromLocal, String localDcName) {
     ReplicaId replicaToReturn = null;

From 61b771421050bdcf0bf054d06a79ab7bb1257b5e Mon Sep 17 00:00:00 2001
From: Yingyi Zhang 
Date: Thu, 16 May 2019 11:09:31 -0700
Subject: [PATCH 3/8] added java doc for two methods

---
 .../main/java/com.github.ambry.router/GetBlobOperation.java | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
index d65908e253..d60ee46c46 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
@@ -300,6 +300,9 @@ void poll(RequestRegistrationCallback requestRegistrationCallback)
     }
   }
 
+  /**
+   * @return the {@link OperationTracker} being used by first chunk.
+   */
   OperationTracker getFirstChunkOperationTrackerInUse() {
     return firstChunk.getChunkOperationTrackerInUse();
   }
@@ -985,6 +988,9 @@ boolean isComplete() {
       return state == ChunkState.Complete;
     }
 
+    /**
+     * @return {@link OperationTracker} being used by this chunk.
+     */
     OperationTracker getChunkOperationTrackerInUse() {
       return chunkOperationTracker;
     }

From 8d5e984669d43a226652538cf4f0e23836ce1442 Mon Sep 17 00:00:00 2001
From: Yingyi Zhang 
Date: Fri, 17 May 2019 10:12:12 -0700
Subject: [PATCH 4/8] introduce RequsetResult to address the comments

---
 .../AdaptiveOperationTracker.java             |  6 +--
 .../DeleteOperation.java                      |  5 +-
 .../GetBlobInfoOperation.java                 |  5 +-
 .../GetBlobOperation.java                     |  5 +-
 .../OperationTracker.java                     | 12 ++---
 .../com.github.ambry.router/PutOperation.java |  4 +-
 .../RequestResult.java                        | 18 +++++++
 .../SimpleOperationTracker.java               |  4 +-
 .../TtlUpdateOperation.java                   |  5 +-
 .../AdaptiveOperationTrackerTest.java         | 10 ++--
 .../OperationTrackerTest.java                 | 50 +++++++++----------
 11 files changed, 72 insertions(+), 52 deletions(-)
 create mode 100644 ambry-router/src/main/java/com.github.ambry.router/RequestResult.java

diff --git a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
index 47ac85f04d..f5019ce2f8 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
@@ -77,15 +77,15 @@ class AdaptiveOperationTracker extends SimpleOperationTracker {
   }
 
   @Override
-  public void onResponse(ReplicaId replicaId, boolean isSuccessFul, RouterErrorCode routerErrorCode) {
-    super.onResponse(replicaId, isSuccessFul, routerErrorCode);
+  public void onResponse(ReplicaId replicaId, RequestResult requestResult) {
+    super.onResponse(replicaId, requestResult);
     long elapsedTime;
     if (unexpiredRequestSendTimes.containsKey(replicaId)) {
       elapsedTime = time.milliseconds() - unexpiredRequestSendTimes.remove(replicaId).getSecond();
     } else {
       elapsedTime = time.milliseconds() - expiredRequestSendTimes.remove(replicaId);
     }
-    if (routerErrorCode != RouterErrorCode.OperationTimedOut) {
+    if (requestResult != RequestResult.TIMED_OUT) {
       getLatencyHistogram(replicaId).update(elapsedTime);
     }
   }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
index c2e4bd09bb..da132b1f03 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
@@ -252,7 +252,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo
     switch (serverErrorCode) {
       case No_Error:
       case Blob_Deleted:
-        operationTracker.onResponse(replica, true, null);
+        operationTracker.onResponse(replica, RequestResult.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, replica)) {
           logger.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(),
               replica.getDataNodeId().getDatacenterName());
@@ -303,7 +303,8 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
         resolvedRouterErrorCode = error;
       }
     }
-    operationTracker.onResponse(replica, false, resolvedRouterErrorCode);
+    operationTracker.onResponse(replica,
+        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? RequestResult.TIMED_OUT : RequestResult.FAILURE);
     if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) {
       routerMetrics.routerRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
index 67d35df9f2..38dbf7a8e1 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
@@ -285,7 +285,7 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon
             MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0);
             MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0);
             handleBody(getResponse.getInputStream(), messageMetadata, messageInfo);
-            operationTracker.onResponse(getRequestInfo.replicaId, true, null);
+            operationTracker.onResponse(getRequestInfo.replicaId, RequestResult.SUCCESS);
             if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) {
               logger.trace("Cross colo request successful for remote replica in {} ",
                   getRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -321,7 +321,8 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon
    * @param routerErrorCode the {@link RouterErrorCode} associated with the failed response.
    */
   void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) {
-    operationTracker.onResponse(replicaId, false, routerErrorCode);
+    operationTracker.onResponse(replicaId,
+        routerErrorCode == RouterErrorCode.OperationTimedOut ? RequestResult.TIMED_OUT : RequestResult.FAILURE);
     routerMetrics.routerRequestErrorCount.inc();
     routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getBlobInfoRequestErrorCount.inc();
   }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
index d60ee46c46..04e126a69f 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
@@ -870,7 +870,7 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g
               MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0);
               MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0);
               handleBody(getResponse.getInputStream(), messageMetadata, messageInfo);
-              chunkOperationTracker.onResponse(getRequestInfo.replicaId, true, null);
+              chunkOperationTracker.onResponse(getRequestInfo.replicaId, RequestResult.SUCCESS);
               if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) {
                 logger.trace("Cross colo request successful for remote replica in {} ",
                     getRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -905,7 +905,8 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g
      * @param routerErrorCode the {@link RouterErrorCode} associated with the failed response.
      */
     void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) {
-      chunkOperationTracker.onResponse(replicaId, false, routerErrorCode);
+      chunkOperationTracker.onResponse(replicaId,
+          routerErrorCode == RouterErrorCode.OperationTimedOut ? RequestResult.TIMED_OUT : RequestResult.FAILURE);
       routerMetrics.routerRequestErrorCount.inc();
       routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
index a1b040f70b..3f62c31206 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
@@ -26,7 +26,7 @@
  * next replica to send a request.
  *
  * When an operation is progressing by receiving responses from replicas, its {@code OperationTracker}
- * needs to be informed by calling {@link #onResponse(ReplicaId, boolean, RouterErrorCode)}.
+ * needs to be informed by calling {@link #onResponse(ReplicaId, RequestResult)}.
  *
  * Typical usage of an {@code OperationTracker} would be:
  * 
@@ -61,14 +61,12 @@ interface OperationTracker {
   boolean isDone();
 
   /**
-   * Accounts for successful or failed response from a replica. Must invoke this method
-   * if a successful or failed response is received for a replica.
+   * Accounts for successful, failed or timed-out response from a replica. Must invoke this method
+   * if a successful/failed/timed-out response is received for a replica.
    * @param replicaId ReplicaId associated with this response.
-   * @param isSuccessful Whether the request to the replicaId is successful or not.
-   * @param routerErrorCode The {@link RouterErrorCode} associated with this request if it failed. This can be null if
-   *                        request is successful.
+   * @param requestResult The result of a single request (SUCCESS, FAILURE or TIMED_OUT).
    */
-  void onResponse(ReplicaId replicaId, boolean isSuccessful, RouterErrorCode routerErrorCode);
+  void onResponse(ReplicaId replicaId, RequestResult requestResult);
 
   /**
    * Provide an iterator to the replicas to which requests may be sent. Each time when start to iterate
diff --git a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
index 56684572b4..3ddc71fe04 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
@@ -1407,7 +1407,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
         }
       }
       if (isSuccessful) {
-        operationTracker.onResponse(chunkPutRequestInfo.replicaId, true, null);
+        operationTracker.onResponse(chunkPutRequestInfo.replicaId, RequestResult.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, chunkPutRequestInfo.replicaId)) {
           logger.trace("Cross colo request successful for remote replica in {} ",
               chunkPutRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -1424,7 +1424,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
      * @param replicaId the {@link ReplicaId} associated with the failed response.
      */
     void onErrorResponse(ReplicaId replicaId) {
-      operationTracker.onResponse(replicaId, false, RouterErrorCode.AmbryUnavailable);
+      operationTracker.onResponse(replicaId, RequestResult.FAILURE);
       routerMetrics.routerRequestErrorCount.inc();
       routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).putRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/RequestResult.java b/ambry-router/src/main/java/com.github.ambry.router/RequestResult.java
new file mode 100644
index 0000000000..9c78d87f2b
--- /dev/null
+++ b/ambry-router/src/main/java/com.github.ambry.router/RequestResult.java
@@ -0,0 +1,18 @@
+/**
+ * Copyright 2019 LinkedIn Corp. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ */
+package com.github.ambry.router;
+
+public enum RequestResult {
+  SUCCESS, FAILURE, TIMED_OUT
+}
diff --git a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
index 98743edf77..cc2e8e26d8 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
@@ -185,9 +185,9 @@ public boolean isDone() {
   }
 
   @Override
-  public void onResponse(ReplicaId replicaId, boolean isSuccessFul, RouterErrorCode routerErrorCode) {
+  public void onResponse(ReplicaId replicaId, RequestResult requestResult) {
     inflightCount--;
-    if (isSuccessFul) {
+    if (requestResult == RequestResult.SUCCESS) {
       succeededCount++;
     } else {
       failedCount++;
diff --git a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
index a65974f580..c4a94822c4 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
@@ -243,7 +243,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo
     switch (serverErrorCode) {
       case No_Error:
       case Blob_Already_Updated:
-        operationTracker.onResponse(replica, true, null);
+        operationTracker.onResponse(replica, RequestResult.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, replica)) {
           LOGGER.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(),
               replica.getDataNodeId().getDatacenterName());
@@ -297,7 +297,8 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
         resolvedRouterErrorCode = error;
       }
     }
-    operationTracker.onResponse(replica, false, resolvedRouterErrorCode);
+    operationTracker.onResponse(replica,
+        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? RequestResult.TIMED_OUT : RequestResult.FAILURE);
     if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) {
       routerMetrics.routerRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
index 0435f9aea0..58b5784794 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
@@ -144,7 +144,7 @@ public void adaptationTest() throws InterruptedException {
     // generate a response for every request and make sure there are no errors
     for (int i = 0; i < REPLICA_COUNT; i++) {
       assertFalse("Operation should not be done", ot.isDone());
-      ot.onResponse(inflightReplicas.poll(), true, null);
+      ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be REPLICA_COUNT - 2
@@ -191,13 +191,13 @@ public void noUnexpiredRequestsTest() throws InterruptedException {
     sendRequests(ot, 1);
     // 1-2-0-0
     // provide a response to the second request that is not a success
-    ot.onResponse(inflightReplicas.pollLast(), false, null);
+    ot.onResponse(inflightReplicas.pollLast(), RequestResult.FAILURE);
     // 1-1-0-1
     assertFalse("Operation should not be done", ot.isDone());
     // should now be able to send one more request
     sendRequests(ot, 1);
     // 0-2-0-1
-    ot.onResponse(inflightReplicas.pollLast(), true, null);
+    ot.onResponse(inflightReplicas.pollLast(), RequestResult.SUCCESS);
     // 0-1-1-1
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -235,7 +235,7 @@ public void trackerUpdateBetweenHasNextAndNextTest() throws InterruptedException
 
     sendRequests(ot, 1);
     // 1-2-0-0
-    ot.onResponse(inflightReplicas.pollLast(), true, null);
+    ot.onResponse(inflightReplicas.pollLast(), RequestResult.SUCCESS);
     // 1-1-1-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -392,7 +392,7 @@ private void verifyHistogramRecording(OperationTracker ot, boolean succeedReques
       Double[] expectedAverages, Histogram tracker) throws InterruptedException {
     for (double expectedAverage : expectedAverages) {
       time.sleep(timeIncrement);
-      ot.onResponse(inflightReplicas.poll(), succeedRequests, null);
+      ot.onResponse(inflightReplicas.poll(), succeedRequests ? RequestResult.SUCCESS : RequestResult.FAILURE);
       assertEquals("Average does not match. Histogram recording may be incorrect", expectedAverage,
           tracker.getSnapshot().getMean(), 0.001);
     }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
index f2bb29d1d7..74bcc81e76 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
@@ -117,13 +117,13 @@ public void localSucceedTest() {
     // 0-3-0-0; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), true, null);
+      ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
     }
     // 0-1-2-0; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
 
-    ot.onResponse(inflightReplicas.poll(), false, null);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
     // 0-0-2-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -146,12 +146,12 @@ public void localFailTest() {
     sendRequests(ot, 3, false);
     // 0-3-0-0; 9-0-0-0
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), false, null);
+      ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
     }
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
     // 0-1-0-2; 9-0-0-0
-    ot.onResponse(inflightReplicas.poll(), true, null);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
     // 0-0-1-2; 9-0-0-0
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -174,14 +174,14 @@ public void localSucceedWithDifferentParameterTest() {
     sendRequests(ot, 2, false);
     // 1-2-0-0; 9-0-0-0
 
-    ot.onResponse(inflightReplicas.poll(), false, null);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
 
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
-    ot.onResponse(inflightReplicas.poll(), true, null);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
     // 0-1-1-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -212,35 +212,35 @@ public void remoteReplicaTest() {
     // 1-2-0-0; 9-0-0-0
     ReplicaId id = inflightReplicas.poll();
     assertEquals("First request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, false, null);
+    ot.onResponse(id, RequestResult.FAILURE);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     id = inflightReplicas.poll();
     assertEquals("Second request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, false, null);
+    ot.onResponse(id, RequestResult.FAILURE);
     id = inflightReplicas.poll();
     assertEquals("Third request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, false, null);
+    ot.onResponse(id, RequestResult.FAILURE);
     // 0-0-0-3; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 7-2-0-0
     assertFalse("Operation should not be done", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), false, null);
+      ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
     }
     // 0-0-0-3; 7-0-0-2
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 5-2-0-2
-    ot.onResponse(inflightReplicas.poll(), false, null);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     // 0-0-0-3; 5-1-0-3
     sendRequests(ot, 1, false);
     // 0-0-0-3; 4-1-0-3
-    ot.onResponse(inflightReplicas.poll(), true, null);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -262,7 +262,7 @@ public void fullSuccessTargetTest() {
     while (!ot.hasSucceeded()) {
       sendRequests(ot, 3, false);
       for (int i = 0; i < 3; i++) {
-        ot.onResponse(inflightReplicas.poll(), true, null);
+        ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
       }
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -295,14 +295,14 @@ public void useReplicaNotSucceededSendTest() {
     localDcName = datanodes.get(0).getDatacenterName();
     OperationTracker ot = getOperationTracker(true, 1, 2, true, Integer.MAX_VALUE);
     sendRequests(ot, 2, true);
-    ot.onResponse(inflightReplicas.poll(), false, null);
-    ot.onResponse(inflightReplicas.poll(), false, null);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, true);
-    ot.onResponse(inflightReplicas.poll(), false, null);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
-    ot.onResponse(inflightReplicas.poll(), true, null);
+    ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -338,12 +338,12 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, false, null);
+      ot.onResponse(replica, RequestResult.FAILURE);
     }
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, false, null);
+      ot.onResponse(replica, RequestResult.FAILURE);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
@@ -351,7 +351,7 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, true, null);
+      ot.onResponse(replica, RequestResult.SUCCESS);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -369,7 +369,7 @@ public void replicasOrderingTestOriginatingIsLocal() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, true, null);
+      ot.onResponse(replica, RequestResult.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -389,14 +389,14 @@ public void replicasOrderingTestOriginatingNotLocal() {
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
       // fail first 3 requests to local
-      ot.onResponse(replica, false, null);
+      ot.onResponse(replica, RequestResult.FAILURE);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, true, null);
+      ot.onResponse(replica, RequestResult.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -418,14 +418,14 @@ public void replicasOrderTestOriginatingDcOnly() {
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
       // fail first 3 requests to local replicas
-      ot.onResponse(replica, false, null);
+      ot.onResponse(replica, RequestResult.FAILURE);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, true, null);
+      ot.onResponse(replica, RequestResult.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());

From d88a815420b46f490e6b2ecabdba8b80c4bb284b Mon Sep 17 00:00:00 2001
From: Yingyi Zhang 
Date: Fri, 17 May 2019 11:12:20 -0700
Subject: [PATCH 5/8] address ze's comments

---
 .../AdaptiveOperationTracker.java             |  6 +--
 .../DeleteOperation.java                      |  5 +-
 .../GetBlobInfoOperation.java                 |  5 +-
 .../GetBlobOperation.java                     |  5 +-
 .../OperationTracker.java                     |  6 +--
 .../com.github.ambry.router/PutOperation.java |  4 +-
 ...sult.java => RouterRequestFinalState.java} |  5 +-
 .../SimpleOperationTracker.java               |  4 +-
 .../TtlUpdateOperation.java                   |  5 +-
 .../AdaptiveOperationTrackerTest.java         | 11 ++--
 .../GetBlobInfoOperationTest.java             |  1 +
 .../GetBlobOperationTest.java                 |  1 +
 .../OperationTrackerTest.java                 | 50 +++++++++----------
 13 files changed, 59 insertions(+), 49 deletions(-)
 rename ambry-router/src/main/java/com.github.ambry.router/{RequestResult.java => RouterRequestFinalState.java} (86%)

diff --git a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
index f5019ce2f8..11d2aa9e3b 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
@@ -77,15 +77,15 @@ class AdaptiveOperationTracker extends SimpleOperationTracker {
   }
 
   @Override
-  public void onResponse(ReplicaId replicaId, RequestResult requestResult) {
-    super.onResponse(replicaId, requestResult);
+  public void onResponse(ReplicaId replicaId, RouterRequestFinalState routerRequestFinalState) {
+    super.onResponse(replicaId, routerRequestFinalState);
     long elapsedTime;
     if (unexpiredRequestSendTimes.containsKey(replicaId)) {
       elapsedTime = time.milliseconds() - unexpiredRequestSendTimes.remove(replicaId).getSecond();
     } else {
       elapsedTime = time.milliseconds() - expiredRequestSendTimes.remove(replicaId);
     }
-    if (requestResult != RequestResult.TIMED_OUT) {
+    if (routerRequestFinalState != RouterRequestFinalState.TIMED_OUT) {
       getLatencyHistogram(replicaId).update(elapsedTime);
     }
   }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
index da132b1f03..51a980d6b3 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
@@ -252,7 +252,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo
     switch (serverErrorCode) {
       case No_Error:
       case Blob_Deleted:
-        operationTracker.onResponse(replica, RequestResult.SUCCESS);
+        operationTracker.onResponse(replica, RouterRequestFinalState.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, replica)) {
           logger.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(),
               replica.getDataNodeId().getDatacenterName());
@@ -304,7 +304,8 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
       }
     }
     operationTracker.onResponse(replica,
-        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? RequestResult.TIMED_OUT : RequestResult.FAILURE);
+        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? RouterRequestFinalState.TIMED_OUT
+            : RouterRequestFinalState.FAILURE);
     if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) {
       routerMetrics.routerRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
index 38dbf7a8e1..34a2cc1e67 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
@@ -285,7 +285,7 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon
             MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0);
             MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0);
             handleBody(getResponse.getInputStream(), messageMetadata, messageInfo);
-            operationTracker.onResponse(getRequestInfo.replicaId, RequestResult.SUCCESS);
+            operationTracker.onResponse(getRequestInfo.replicaId, RouterRequestFinalState.SUCCESS);
             if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) {
               logger.trace("Cross colo request successful for remote replica in {} ",
                   getRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -322,7 +322,8 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon
    */
   void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) {
     operationTracker.onResponse(replicaId,
-        routerErrorCode == RouterErrorCode.OperationTimedOut ? RequestResult.TIMED_OUT : RequestResult.FAILURE);
+        routerErrorCode == RouterErrorCode.OperationTimedOut ? RouterRequestFinalState.TIMED_OUT
+            : RouterRequestFinalState.FAILURE);
     routerMetrics.routerRequestErrorCount.inc();
     routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getBlobInfoRequestErrorCount.inc();
   }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
index 04e126a69f..5d7d43a2e7 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
@@ -870,7 +870,7 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g
               MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0);
               MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0);
               handleBody(getResponse.getInputStream(), messageMetadata, messageInfo);
-              chunkOperationTracker.onResponse(getRequestInfo.replicaId, RequestResult.SUCCESS);
+              chunkOperationTracker.onResponse(getRequestInfo.replicaId, RouterRequestFinalState.SUCCESS);
               if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) {
                 logger.trace("Cross colo request successful for remote replica in {} ",
                     getRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -906,7 +906,8 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g
      */
     void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) {
       chunkOperationTracker.onResponse(replicaId,
-          routerErrorCode == RouterErrorCode.OperationTimedOut ? RequestResult.TIMED_OUT : RequestResult.FAILURE);
+          routerErrorCode == RouterErrorCode.OperationTimedOut ? RouterRequestFinalState.TIMED_OUT
+              : RouterRequestFinalState.FAILURE);
       routerMetrics.routerRequestErrorCount.inc();
       routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
index 3f62c31206..722f1f8b43 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
@@ -26,7 +26,7 @@
  * next replica to send a request.
  *
  * When an operation is progressing by receiving responses from replicas, its {@code OperationTracker}
- * needs to be informed by calling {@link #onResponse(ReplicaId, RequestResult)}.
+ * needs to be informed by calling {@link #onResponse(ReplicaId, RouterRequestFinalState)}.
  *
  * Typical usage of an {@code OperationTracker} would be:
  * 
@@ -64,9 +64,9 @@ interface OperationTracker {
    * Accounts for successful, failed or timed-out response from a replica. Must invoke this method
    * if a successful/failed/timed-out response is received for a replica.
    * @param replicaId ReplicaId associated with this response.
-   * @param requestResult The result of a single request (SUCCESS, FAILURE or TIMED_OUT).
+   * @param routerRequestFinalState The result of a single request (SUCCESS, FAILURE or TIMED_OUT).
    */
-  void onResponse(ReplicaId replicaId, RequestResult requestResult);
+  void onResponse(ReplicaId replicaId, RouterRequestFinalState routerRequestFinalState);
 
   /**
    * Provide an iterator to the replicas to which requests may be sent. Each time when start to iterate
diff --git a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
index 3ddc71fe04..f96b7107d7 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
@@ -1407,7 +1407,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
         }
       }
       if (isSuccessful) {
-        operationTracker.onResponse(chunkPutRequestInfo.replicaId, RequestResult.SUCCESS);
+        operationTracker.onResponse(chunkPutRequestInfo.replicaId, RouterRequestFinalState.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, chunkPutRequestInfo.replicaId)) {
           logger.trace("Cross colo request successful for remote replica in {} ",
               chunkPutRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -1424,7 +1424,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
      * @param replicaId the {@link ReplicaId} associated with the failed response.
      */
     void onErrorResponse(ReplicaId replicaId) {
-      operationTracker.onResponse(replicaId, RequestResult.FAILURE);
+      operationTracker.onResponse(replicaId, RouterRequestFinalState.FAILURE);
       routerMetrics.routerRequestErrorCount.inc();
       routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).putRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/RequestResult.java b/ambry-router/src/main/java/com.github.ambry.router/RouterRequestFinalState.java
similarity index 86%
rename from ambry-router/src/main/java/com.github.ambry.router/RequestResult.java
rename to ambry-router/src/main/java/com.github.ambry.router/RouterRequestFinalState.java
index 9c78d87f2b..04f11ab2b4 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/RequestResult.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/RouterRequestFinalState.java
@@ -13,6 +13,9 @@
  */
 package com.github.ambry.router;
 
-public enum RequestResult {
+/**
+ * The final state of a single router request.
+ */
+public enum RouterRequestFinalState {
   SUCCESS, FAILURE, TIMED_OUT
 }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
index cc2e8e26d8..ada4f1bb44 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
@@ -185,9 +185,9 @@ public boolean isDone() {
   }
 
   @Override
-  public void onResponse(ReplicaId replicaId, RequestResult requestResult) {
+  public void onResponse(ReplicaId replicaId, RouterRequestFinalState routerRequestFinalState) {
     inflightCount--;
-    if (requestResult == RequestResult.SUCCESS) {
+    if (routerRequestFinalState == RouterRequestFinalState.SUCCESS) {
       succeededCount++;
     } else {
       failedCount++;
diff --git a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
index c4a94822c4..f52f251c2f 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
@@ -243,7 +243,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo
     switch (serverErrorCode) {
       case No_Error:
       case Blob_Already_Updated:
-        operationTracker.onResponse(replica, RequestResult.SUCCESS);
+        operationTracker.onResponse(replica, RouterRequestFinalState.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, replica)) {
           LOGGER.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(),
               replica.getDataNodeId().getDatacenterName());
@@ -298,7 +298,8 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
       }
     }
     operationTracker.onResponse(replica,
-        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? RequestResult.TIMED_OUT : RequestResult.FAILURE);
+        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? RouterRequestFinalState.TIMED_OUT
+            : RouterRequestFinalState.FAILURE);
     if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) {
       routerMetrics.routerRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
index 58b5784794..89b61ebc66 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
@@ -144,7 +144,7 @@ public void adaptationTest() throws InterruptedException {
     // generate a response for every request and make sure there are no errors
     for (int i = 0; i < REPLICA_COUNT; i++) {
       assertFalse("Operation should not be done", ot.isDone());
-      ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
+      ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be REPLICA_COUNT - 2
@@ -191,13 +191,13 @@ public void noUnexpiredRequestsTest() throws InterruptedException {
     sendRequests(ot, 1);
     // 1-2-0-0
     // provide a response to the second request that is not a success
-    ot.onResponse(inflightReplicas.pollLast(), RequestResult.FAILURE);
+    ot.onResponse(inflightReplicas.pollLast(), RouterRequestFinalState.FAILURE);
     // 1-1-0-1
     assertFalse("Operation should not be done", ot.isDone());
     // should now be able to send one more request
     sendRequests(ot, 1);
     // 0-2-0-1
-    ot.onResponse(inflightReplicas.pollLast(), RequestResult.SUCCESS);
+    ot.onResponse(inflightReplicas.pollLast(), RouterRequestFinalState.SUCCESS);
     // 0-1-1-1
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -235,7 +235,7 @@ public void trackerUpdateBetweenHasNextAndNextTest() throws InterruptedException
 
     sendRequests(ot, 1);
     // 1-2-0-0
-    ot.onResponse(inflightReplicas.pollLast(), RequestResult.SUCCESS);
+    ot.onResponse(inflightReplicas.pollLast(), RouterRequestFinalState.SUCCESS);
     // 1-1-1-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -392,7 +392,8 @@ private void verifyHistogramRecording(OperationTracker ot, boolean succeedReques
       Double[] expectedAverages, Histogram tracker) throws InterruptedException {
     for (double expectedAverage : expectedAverages) {
       time.sleep(timeIncrement);
-      ot.onResponse(inflightReplicas.poll(), succeedRequests ? RequestResult.SUCCESS : RequestResult.FAILURE);
+      ot.onResponse(inflightReplicas.poll(),
+          succeedRequests ? RouterRequestFinalState.SUCCESS : RouterRequestFinalState.FAILURE);
       assertEquals("Average does not match. Histogram recording may be incorrect", expectedAverage,
           tracker.getSnapshot().getMean(), 0.001);
     }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java b/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
index 449036362a..e074fb9e70 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
@@ -651,6 +651,7 @@ private Properties getNonBlockingRouterProperties() {
     properties.setProperty("router.get.request.parallelism", Integer.toString(requestParallelism));
     properties.setProperty("router.get.success.target", Integer.toString(successTarget));
     properties.setProperty("router.get.operation.tracker.type", operationTrackerType);
+    properties.setProperty("router.request.timeout.ms", Integer.toString(200));
     return properties;
   }
 }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java b/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
index ce13bd3e34..40050748f2 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
@@ -1486,6 +1486,7 @@ private Properties getDefaultNonBlockingRouterProperties() {
     properties.setProperty("router.get.request.parallelism", Integer.toString(2));
     properties.setProperty("router.get.success.target", Integer.toString(1));
     properties.setProperty("router.get.operation.tracker.type", operationTrackerType);
+    properties.setProperty("router.request.timeout.ms", Integer.toString(200));
     return properties;
   }
 }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
index 74bcc81e76..b66b3ff953 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
@@ -117,13 +117,13 @@ public void localSucceedTest() {
     // 0-3-0-0; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
+      ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
     }
     // 0-1-2-0; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
 
-    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
     // 0-0-2-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -146,12 +146,12 @@ public void localFailTest() {
     sendRequests(ot, 3, false);
     // 0-3-0-0; 9-0-0-0
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
+      ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
     }
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
     // 0-1-0-2; 9-0-0-0
-    ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
     // 0-0-1-2; 9-0-0-0
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -174,14 +174,14 @@ public void localSucceedWithDifferentParameterTest() {
     sendRequests(ot, 2, false);
     // 1-2-0-0; 9-0-0-0
 
-    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
 
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
-    ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
     // 0-1-1-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -212,35 +212,35 @@ public void remoteReplicaTest() {
     // 1-2-0-0; 9-0-0-0
     ReplicaId id = inflightReplicas.poll();
     assertEquals("First request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, RequestResult.FAILURE);
+    ot.onResponse(id, RouterRequestFinalState.FAILURE);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     id = inflightReplicas.poll();
     assertEquals("Second request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, RequestResult.FAILURE);
+    ot.onResponse(id, RouterRequestFinalState.FAILURE);
     id = inflightReplicas.poll();
     assertEquals("Third request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, RequestResult.FAILURE);
+    ot.onResponse(id, RouterRequestFinalState.FAILURE);
     // 0-0-0-3; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 7-2-0-0
     assertFalse("Operation should not be done", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
+      ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
     }
     // 0-0-0-3; 7-0-0-2
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 5-2-0-2
-    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     // 0-0-0-3; 5-1-0-3
     sendRequests(ot, 1, false);
     // 0-0-0-3; 4-1-0-3
-    ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -262,7 +262,7 @@ public void fullSuccessTargetTest() {
     while (!ot.hasSucceeded()) {
       sendRequests(ot, 3, false);
       for (int i = 0; i < 3; i++) {
-        ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
+        ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
       }
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -295,14 +295,14 @@ public void useReplicaNotSucceededSendTest() {
     localDcName = datanodes.get(0).getDatacenterName();
     OperationTracker ot = getOperationTracker(true, 1, 2, true, Integer.MAX_VALUE);
     sendRequests(ot, 2, true);
-    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
-    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, true);
-    ot.onResponse(inflightReplicas.poll(), RequestResult.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
-    ot.onResponse(inflightReplicas.poll(), RequestResult.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -338,12 +338,12 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RequestResult.FAILURE);
+      ot.onResponse(replica, RouterRequestFinalState.FAILURE);
     }
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RequestResult.FAILURE);
+      ot.onResponse(replica, RouterRequestFinalState.FAILURE);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
@@ -351,7 +351,7 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RequestResult.SUCCESS);
+      ot.onResponse(replica, RouterRequestFinalState.SUCCESS);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -369,7 +369,7 @@ public void replicasOrderingTestOriginatingIsLocal() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RequestResult.SUCCESS);
+      ot.onResponse(replica, RouterRequestFinalState.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -389,14 +389,14 @@ public void replicasOrderingTestOriginatingNotLocal() {
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
       // fail first 3 requests to local
-      ot.onResponse(replica, RequestResult.FAILURE);
+      ot.onResponse(replica, RouterRequestFinalState.FAILURE);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RequestResult.SUCCESS);
+      ot.onResponse(replica, RouterRequestFinalState.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -418,14 +418,14 @@ public void replicasOrderTestOriginatingDcOnly() {
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
       // fail first 3 requests to local replicas
-      ot.onResponse(replica, RequestResult.FAILURE);
+      ot.onResponse(replica, RouterRequestFinalState.FAILURE);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RequestResult.SUCCESS);
+      ot.onResponse(replica, RouterRequestFinalState.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());

From c04c41c8fae0c0de84c338157dbf72199d20d6b0 Mon Sep 17 00:00:00 2001
From: Yingyi Zhang 
Date: Fri, 17 May 2019 11:29:27 -0700
Subject: [PATCH 6/8] maker timeout in test even shorter

---
 .../java/com.github.ambry.router/GetBlobInfoOperationTest.java  | 2 +-
 .../test/java/com.github.ambry.router/GetBlobOperationTest.java | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java b/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
index e074fb9e70..be28cd179e 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/GetBlobInfoOperationTest.java
@@ -651,7 +651,7 @@ private Properties getNonBlockingRouterProperties() {
     properties.setProperty("router.get.request.parallelism", Integer.toString(requestParallelism));
     properties.setProperty("router.get.success.target", Integer.toString(successTarget));
     properties.setProperty("router.get.operation.tracker.type", operationTrackerType);
-    properties.setProperty("router.request.timeout.ms", Integer.toString(200));
+    properties.setProperty("router.request.timeout.ms", Integer.toString(20));
     return properties;
   }
 }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java b/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
index 40050748f2..0d9a5b0e41 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/GetBlobOperationTest.java
@@ -1486,7 +1486,7 @@ private Properties getDefaultNonBlockingRouterProperties() {
     properties.setProperty("router.get.request.parallelism", Integer.toString(2));
     properties.setProperty("router.get.success.target", Integer.toString(1));
     properties.setProperty("router.get.operation.tracker.type", operationTrackerType);
-    properties.setProperty("router.request.timeout.ms", Integer.toString(200));
+    properties.setProperty("router.request.timeout.ms", Integer.toString(20));
     return properties;
   }
 }

From 9cbafab8278f24bf0149aee14911c5b444ed46f2 Mon Sep 17 00:00:00 2001
From: Yingyi Zhang 
Date: Fri, 17 May 2019 11:59:07 -0700
Subject: [PATCH 7/8] rename the enum name as suggested

---
 .../AdaptiveOperationTracker.java             |  6 +--
 .../DeleteOperation.java                      |  6 +--
 .../GetBlobInfoOperation.java                 |  6 +--
 .../GetBlobOperation.java                     |  6 +--
 .../OperationTracker.java                     |  6 +--
 .../com.github.ambry.router/PutOperation.java |  4 +-
 ...tate.java => ServerRequestFinalState.java} |  4 +-
 .../SimpleOperationTracker.java               |  4 +-
 .../TtlUpdateOperation.java                   |  6 +--
 .../AdaptiveOperationTrackerTest.java         | 10 ++--
 .../OperationTrackerTest.java                 | 50 +++++++++----------
 11 files changed, 54 insertions(+), 54 deletions(-)
 rename ambry-router/src/main/java/com.github.ambry.router/{RouterRequestFinalState.java => ServerRequestFinalState.java} (84%)

diff --git a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
index 11d2aa9e3b..ab6c1a0b45 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
@@ -77,15 +77,15 @@ class AdaptiveOperationTracker extends SimpleOperationTracker {
   }
 
   @Override
-  public void onResponse(ReplicaId replicaId, RouterRequestFinalState routerRequestFinalState) {
-    super.onResponse(replicaId, routerRequestFinalState);
+  public void onResponse(ReplicaId replicaId, ServerRequestFinalState serverRequestFinalState) {
+    super.onResponse(replicaId, serverRequestFinalState);
     long elapsedTime;
     if (unexpiredRequestSendTimes.containsKey(replicaId)) {
       elapsedTime = time.milliseconds() - unexpiredRequestSendTimes.remove(replicaId).getSecond();
     } else {
       elapsedTime = time.milliseconds() - expiredRequestSendTimes.remove(replicaId);
     }
-    if (routerRequestFinalState != RouterRequestFinalState.TIMED_OUT) {
+    if (serverRequestFinalState != ServerRequestFinalState.TIMED_OUT) {
       getLatencyHistogram(replicaId).update(elapsedTime);
     }
   }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
index 51a980d6b3..d4693ab306 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
@@ -252,7 +252,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo
     switch (serverErrorCode) {
       case No_Error:
       case Blob_Deleted:
-        operationTracker.onResponse(replica, RouterRequestFinalState.SUCCESS);
+        operationTracker.onResponse(replica, ServerRequestFinalState.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, replica)) {
           logger.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(),
               replica.getDataNodeId().getDatacenterName());
@@ -304,8 +304,8 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
       }
     }
     operationTracker.onResponse(replica,
-        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? RouterRequestFinalState.TIMED_OUT
-            : RouterRequestFinalState.FAILURE);
+        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? ServerRequestFinalState.TIMED_OUT
+            : ServerRequestFinalState.FAILURE);
     if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) {
       routerMetrics.routerRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
index 34a2cc1e67..f2f8c22081 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
@@ -285,7 +285,7 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon
             MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0);
             MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0);
             handleBody(getResponse.getInputStream(), messageMetadata, messageInfo);
-            operationTracker.onResponse(getRequestInfo.replicaId, RouterRequestFinalState.SUCCESS);
+            operationTracker.onResponse(getRequestInfo.replicaId, ServerRequestFinalState.SUCCESS);
             if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) {
               logger.trace("Cross colo request successful for remote replica in {} ",
                   getRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -322,8 +322,8 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon
    */
   void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) {
     operationTracker.onResponse(replicaId,
-        routerErrorCode == RouterErrorCode.OperationTimedOut ? RouterRequestFinalState.TIMED_OUT
-            : RouterRequestFinalState.FAILURE);
+        routerErrorCode == RouterErrorCode.OperationTimedOut ? ServerRequestFinalState.TIMED_OUT
+            : ServerRequestFinalState.FAILURE);
     routerMetrics.routerRequestErrorCount.inc();
     routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getBlobInfoRequestErrorCount.inc();
   }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
index 5d7d43a2e7..82aef48693 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
@@ -870,7 +870,7 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g
               MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0);
               MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0);
               handleBody(getResponse.getInputStream(), messageMetadata, messageInfo);
-              chunkOperationTracker.onResponse(getRequestInfo.replicaId, RouterRequestFinalState.SUCCESS);
+              chunkOperationTracker.onResponse(getRequestInfo.replicaId, ServerRequestFinalState.SUCCESS);
               if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) {
                 logger.trace("Cross colo request successful for remote replica in {} ",
                     getRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -906,8 +906,8 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g
      */
     void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) {
       chunkOperationTracker.onResponse(replicaId,
-          routerErrorCode == RouterErrorCode.OperationTimedOut ? RouterRequestFinalState.TIMED_OUT
-              : RouterRequestFinalState.FAILURE);
+          routerErrorCode == RouterErrorCode.OperationTimedOut ? ServerRequestFinalState.TIMED_OUT
+              : ServerRequestFinalState.FAILURE);
       routerMetrics.routerRequestErrorCount.inc();
       routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
index 722f1f8b43..ca2a0ef09b 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
@@ -26,7 +26,7 @@
  * next replica to send a request.
  *
  * When an operation is progressing by receiving responses from replicas, its {@code OperationTracker}
- * needs to be informed by calling {@link #onResponse(ReplicaId, RouterRequestFinalState)}.
+ * needs to be informed by calling {@link #onResponse(ReplicaId, ServerRequestFinalState)}.
  *
  * Typical usage of an {@code OperationTracker} would be:
  * 
@@ -64,9 +64,9 @@ interface OperationTracker {
    * Accounts for successful, failed or timed-out response from a replica. Must invoke this method
    * if a successful/failed/timed-out response is received for a replica.
    * @param replicaId ReplicaId associated with this response.
-   * @param routerRequestFinalState The result of a single request (SUCCESS, FAILURE or TIMED_OUT).
+   * @param serverRequestFinalState The result of a single request (SUCCESS, FAILURE or TIMED_OUT).
    */
-  void onResponse(ReplicaId replicaId, RouterRequestFinalState routerRequestFinalState);
+  void onResponse(ReplicaId replicaId, ServerRequestFinalState serverRequestFinalState);
 
   /**
    * Provide an iterator to the replicas to which requests may be sent. Each time when start to iterate
diff --git a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
index f96b7107d7..1eba3acdde 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
@@ -1407,7 +1407,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
         }
       }
       if (isSuccessful) {
-        operationTracker.onResponse(chunkPutRequestInfo.replicaId, RouterRequestFinalState.SUCCESS);
+        operationTracker.onResponse(chunkPutRequestInfo.replicaId, ServerRequestFinalState.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, chunkPutRequestInfo.replicaId)) {
           logger.trace("Cross colo request successful for remote replica in {} ",
               chunkPutRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -1424,7 +1424,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
      * @param replicaId the {@link ReplicaId} associated with the failed response.
      */
     void onErrorResponse(ReplicaId replicaId) {
-      operationTracker.onResponse(replicaId, RouterRequestFinalState.FAILURE);
+      operationTracker.onResponse(replicaId, ServerRequestFinalState.FAILURE);
       routerMetrics.routerRequestErrorCount.inc();
       routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).putRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/RouterRequestFinalState.java b/ambry-router/src/main/java/com.github.ambry.router/ServerRequestFinalState.java
similarity index 84%
rename from ambry-router/src/main/java/com.github.ambry.router/RouterRequestFinalState.java
rename to ambry-router/src/main/java/com.github.ambry.router/ServerRequestFinalState.java
index 04f11ab2b4..66e83f331d 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/RouterRequestFinalState.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/ServerRequestFinalState.java
@@ -14,8 +14,8 @@
 package com.github.ambry.router;
 
 /**
- * The final state of a single router request.
+ * The final state of a single request created to be sent to server.
  */
-public enum RouterRequestFinalState {
+public enum ServerRequestFinalState {
   SUCCESS, FAILURE, TIMED_OUT
 }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
index ada4f1bb44..859874c2ff 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
@@ -185,9 +185,9 @@ public boolean isDone() {
   }
 
   @Override
-  public void onResponse(ReplicaId replicaId, RouterRequestFinalState routerRequestFinalState) {
+  public void onResponse(ReplicaId replicaId, ServerRequestFinalState serverRequestFinalState) {
     inflightCount--;
-    if (routerRequestFinalState == RouterRequestFinalState.SUCCESS) {
+    if (serverRequestFinalState == ServerRequestFinalState.SUCCESS) {
       succeededCount++;
     } else {
       failedCount++;
diff --git a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
index f52f251c2f..917ac06d2f 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
@@ -243,7 +243,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo
     switch (serverErrorCode) {
       case No_Error:
       case Blob_Already_Updated:
-        operationTracker.onResponse(replica, RouterRequestFinalState.SUCCESS);
+        operationTracker.onResponse(replica, ServerRequestFinalState.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, replica)) {
           LOGGER.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(),
               replica.getDataNodeId().getDatacenterName());
@@ -298,8 +298,8 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
       }
     }
     operationTracker.onResponse(replica,
-        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? RouterRequestFinalState.TIMED_OUT
-            : RouterRequestFinalState.FAILURE);
+        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? ServerRequestFinalState.TIMED_OUT
+            : ServerRequestFinalState.FAILURE);
     if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) {
       routerMetrics.routerRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
index 89b61ebc66..70da82e468 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
@@ -144,7 +144,7 @@ public void adaptationTest() throws InterruptedException {
     // generate a response for every request and make sure there are no errors
     for (int i = 0; i < REPLICA_COUNT; i++) {
       assertFalse("Operation should not be done", ot.isDone());
-      ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
+      ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be REPLICA_COUNT - 2
@@ -191,13 +191,13 @@ public void noUnexpiredRequestsTest() throws InterruptedException {
     sendRequests(ot, 1);
     // 1-2-0-0
     // provide a response to the second request that is not a success
-    ot.onResponse(inflightReplicas.pollLast(), RouterRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.pollLast(), ServerRequestFinalState.FAILURE);
     // 1-1-0-1
     assertFalse("Operation should not be done", ot.isDone());
     // should now be able to send one more request
     sendRequests(ot, 1);
     // 0-2-0-1
-    ot.onResponse(inflightReplicas.pollLast(), RouterRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.pollLast(), ServerRequestFinalState.SUCCESS);
     // 0-1-1-1
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -235,7 +235,7 @@ public void trackerUpdateBetweenHasNextAndNextTest() throws InterruptedException
 
     sendRequests(ot, 1);
     // 1-2-0-0
-    ot.onResponse(inflightReplicas.pollLast(), RouterRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.pollLast(), ServerRequestFinalState.SUCCESS);
     // 1-1-1-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -393,7 +393,7 @@ private void verifyHistogramRecording(OperationTracker ot, boolean succeedReques
     for (double expectedAverage : expectedAverages) {
       time.sleep(timeIncrement);
       ot.onResponse(inflightReplicas.poll(),
-          succeedRequests ? RouterRequestFinalState.SUCCESS : RouterRequestFinalState.FAILURE);
+          succeedRequests ? ServerRequestFinalState.SUCCESS : ServerRequestFinalState.FAILURE);
       assertEquals("Average does not match. Histogram recording may be incorrect", expectedAverage,
           tracker.getSnapshot().getMean(), 0.001);
     }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
index b66b3ff953..12a18a4866 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
@@ -117,13 +117,13 @@ public void localSucceedTest() {
     // 0-3-0-0; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
+      ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
     }
     // 0-1-2-0; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
 
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
     // 0-0-2-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -146,12 +146,12 @@ public void localFailTest() {
     sendRequests(ot, 3, false);
     // 0-3-0-0; 9-0-0-0
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
+      ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
     }
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
     // 0-1-0-2; 9-0-0-0
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
     // 0-0-1-2; 9-0-0-0
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -174,14 +174,14 @@ public void localSucceedWithDifferentParameterTest() {
     sendRequests(ot, 2, false);
     // 1-2-0-0; 9-0-0-0
 
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
 
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
     // 0-1-1-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -212,35 +212,35 @@ public void remoteReplicaTest() {
     // 1-2-0-0; 9-0-0-0
     ReplicaId id = inflightReplicas.poll();
     assertEquals("First request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, RouterRequestFinalState.FAILURE);
+    ot.onResponse(id, ServerRequestFinalState.FAILURE);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     id = inflightReplicas.poll();
     assertEquals("Second request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, RouterRequestFinalState.FAILURE);
+    ot.onResponse(id, ServerRequestFinalState.FAILURE);
     id = inflightReplicas.poll();
     assertEquals("Third request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, RouterRequestFinalState.FAILURE);
+    ot.onResponse(id, ServerRequestFinalState.FAILURE);
     // 0-0-0-3; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 7-2-0-0
     assertFalse("Operation should not be done", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
+      ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
     }
     // 0-0-0-3; 7-0-0-2
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 5-2-0-2
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     // 0-0-0-3; 5-1-0-3
     sendRequests(ot, 1, false);
     // 0-0-0-3; 4-1-0-3
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -262,7 +262,7 @@ public void fullSuccessTargetTest() {
     while (!ot.hasSucceeded()) {
       sendRequests(ot, 3, false);
       for (int i = 0; i < 3; i++) {
-        ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
+        ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
       }
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -295,14 +295,14 @@ public void useReplicaNotSucceededSendTest() {
     localDcName = datanodes.get(0).getDatacenterName();
     OperationTracker ot = getOperationTracker(true, 1, 2, true, Integer.MAX_VALUE);
     sendRequests(ot, 2, true);
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, true);
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
-    ot.onResponse(inflightReplicas.poll(), RouterRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -338,12 +338,12 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RouterRequestFinalState.FAILURE);
+      ot.onResponse(replica, ServerRequestFinalState.FAILURE);
     }
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RouterRequestFinalState.FAILURE);
+      ot.onResponse(replica, ServerRequestFinalState.FAILURE);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
@@ -351,7 +351,7 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RouterRequestFinalState.SUCCESS);
+      ot.onResponse(replica, ServerRequestFinalState.SUCCESS);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -369,7 +369,7 @@ public void replicasOrderingTestOriginatingIsLocal() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RouterRequestFinalState.SUCCESS);
+      ot.onResponse(replica, ServerRequestFinalState.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -389,14 +389,14 @@ public void replicasOrderingTestOriginatingNotLocal() {
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
       // fail first 3 requests to local
-      ot.onResponse(replica, RouterRequestFinalState.FAILURE);
+      ot.onResponse(replica, ServerRequestFinalState.FAILURE);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RouterRequestFinalState.SUCCESS);
+      ot.onResponse(replica, ServerRequestFinalState.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -418,14 +418,14 @@ public void replicasOrderTestOriginatingDcOnly() {
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
       // fail first 3 requests to local replicas
-      ot.onResponse(replica, RouterRequestFinalState.FAILURE);
+      ot.onResponse(replica, ServerRequestFinalState.FAILURE);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, RouterRequestFinalState.SUCCESS);
+      ot.onResponse(replica, ServerRequestFinalState.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());

From bc9af854e5be6b498c04034fe9b3349257cc701b Mon Sep 17 00:00:00 2001
From: Yingyi Zhang 
Date: Fri, 17 May 2019 13:06:04 -0700
Subject: [PATCH 8/8] rename the enum to TrackedRequestFinalState to make it
 clear

---
 .../AdaptiveOperationTracker.java             |  6 +--
 .../DeleteOperation.java                      |  6 +--
 .../GetBlobInfoOperation.java                 |  6 +--
 .../GetBlobOperation.java                     |  6 +--
 .../OperationTracker.java                     |  6 +--
 .../com.github.ambry.router/PutOperation.java |  4 +-
 .../SimpleOperationTracker.java               |  4 +-
 ...ate.java => TrackedRequestFinalState.java} |  6 ++-
 .../TtlUpdateOperation.java                   |  6 +--
 .../AdaptiveOperationTrackerTest.java         | 10 ++--
 .../OperationTrackerTest.java                 | 50 +++++++++----------
 11 files changed, 56 insertions(+), 54 deletions(-)
 rename ambry-router/src/main/java/com.github.ambry.router/{ServerRequestFinalState.java => TrackedRequestFinalState.java} (59%)

diff --git a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
index ab6c1a0b45..a7d9fcf314 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/AdaptiveOperationTracker.java
@@ -77,15 +77,15 @@ class AdaptiveOperationTracker extends SimpleOperationTracker {
   }
 
   @Override
-  public void onResponse(ReplicaId replicaId, ServerRequestFinalState serverRequestFinalState) {
-    super.onResponse(replicaId, serverRequestFinalState);
+  public void onResponse(ReplicaId replicaId, TrackedRequestFinalState trackedRequestFinalState) {
+    super.onResponse(replicaId, trackedRequestFinalState);
     long elapsedTime;
     if (unexpiredRequestSendTimes.containsKey(replicaId)) {
       elapsedTime = time.milliseconds() - unexpiredRequestSendTimes.remove(replicaId).getSecond();
     } else {
       elapsedTime = time.milliseconds() - expiredRequestSendTimes.remove(replicaId);
     }
-    if (serverRequestFinalState != ServerRequestFinalState.TIMED_OUT) {
+    if (trackedRequestFinalState != TrackedRequestFinalState.TIMED_OUT) {
       getLatencyHistogram(replicaId).update(elapsedTime);
     }
   }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
index d4693ab306..c7f0677d7a 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/DeleteOperation.java
@@ -252,7 +252,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo
     switch (serverErrorCode) {
       case No_Error:
       case Blob_Deleted:
-        operationTracker.onResponse(replica, ServerRequestFinalState.SUCCESS);
+        operationTracker.onResponse(replica, TrackedRequestFinalState.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, replica)) {
           logger.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(),
               replica.getDataNodeId().getDatacenterName());
@@ -304,8 +304,8 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
       }
     }
     operationTracker.onResponse(replica,
-        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? ServerRequestFinalState.TIMED_OUT
-            : ServerRequestFinalState.FAILURE);
+        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? TrackedRequestFinalState.TIMED_OUT
+            : TrackedRequestFinalState.FAILURE);
     if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) {
       routerMetrics.routerRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
index f2f8c22081..34949ed8fe 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobInfoOperation.java
@@ -285,7 +285,7 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon
             MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0);
             MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0);
             handleBody(getResponse.getInputStream(), messageMetadata, messageInfo);
-            operationTracker.onResponse(getRequestInfo.replicaId, ServerRequestFinalState.SUCCESS);
+            operationTracker.onResponse(getRequestInfo.replicaId, TrackedRequestFinalState.SUCCESS);
             if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) {
               logger.trace("Cross colo request successful for remote replica in {} ",
                   getRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -322,8 +322,8 @@ private void processGetBlobInfoResponse(GetRequestInfo getRequestInfo, GetRespon
    */
   void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) {
     operationTracker.onResponse(replicaId,
-        routerErrorCode == RouterErrorCode.OperationTimedOut ? ServerRequestFinalState.TIMED_OUT
-            : ServerRequestFinalState.FAILURE);
+        routerErrorCode == RouterErrorCode.OperationTimedOut ? TrackedRequestFinalState.TIMED_OUT
+            : TrackedRequestFinalState.FAILURE);
     routerMetrics.routerRequestErrorCount.inc();
     routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getBlobInfoRequestErrorCount.inc();
   }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
index 82aef48693..b1460186f1 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/GetBlobOperation.java
@@ -870,7 +870,7 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g
               MessageMetadata messageMetadata = partitionResponseInfo.getMessageMetadataList().get(0);
               MessageInfo messageInfo = partitionResponseInfo.getMessageInfoList().get(0);
               handleBody(getResponse.getInputStream(), messageMetadata, messageInfo);
-              chunkOperationTracker.onResponse(getRequestInfo.replicaId, ServerRequestFinalState.SUCCESS);
+              chunkOperationTracker.onResponse(getRequestInfo.replicaId, TrackedRequestFinalState.SUCCESS);
               if (RouterUtils.isRemoteReplica(routerConfig, getRequestInfo.replicaId)) {
                 logger.trace("Cross colo request successful for remote replica in {} ",
                     getRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -906,8 +906,8 @@ private void processGetBlobResponse(GetRequestInfo getRequestInfo, GetResponse g
      */
     void onErrorResponse(ReplicaId replicaId, RouterErrorCode routerErrorCode) {
       chunkOperationTracker.onResponse(replicaId,
-          routerErrorCode == RouterErrorCode.OperationTimedOut ? ServerRequestFinalState.TIMED_OUT
-              : ServerRequestFinalState.FAILURE);
+          routerErrorCode == RouterErrorCode.OperationTimedOut ? TrackedRequestFinalState.TIMED_OUT
+              : TrackedRequestFinalState.FAILURE);
       routerMetrics.routerRequestErrorCount.inc();
       routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).getRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
index ca2a0ef09b..00ca68bcfa 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/OperationTracker.java
@@ -26,7 +26,7 @@
  * next replica to send a request.
  *
  * When an operation is progressing by receiving responses from replicas, its {@code OperationTracker}
- * needs to be informed by calling {@link #onResponse(ReplicaId, ServerRequestFinalState)}.
+ * needs to be informed by calling {@link #onResponse(ReplicaId, TrackedRequestFinalState)}.
  *
  * Typical usage of an {@code OperationTracker} would be:
  * 
@@ -64,9 +64,9 @@ interface OperationTracker {
    * Accounts for successful, failed or timed-out response from a replica. Must invoke this method
    * if a successful/failed/timed-out response is received for a replica.
    * @param replicaId ReplicaId associated with this response.
-   * @param serverRequestFinalState The result of a single request (SUCCESS, FAILURE or TIMED_OUT).
+   * @param trackedRequestFinalState The final state of a single request being tracked (SUCCESS, FAILURE or TIMED_OUT).
    */
-  void onResponse(ReplicaId replicaId, ServerRequestFinalState serverRequestFinalState);
+  void onResponse(ReplicaId replicaId, TrackedRequestFinalState trackedRequestFinalState);
 
   /**
    * Provide an iterator to the replicas to which requests may be sent. Each time when start to iterate
diff --git a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
index 1eba3acdde..37f24c93aa 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/PutOperation.java
@@ -1407,7 +1407,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
         }
       }
       if (isSuccessful) {
-        operationTracker.onResponse(chunkPutRequestInfo.replicaId, ServerRequestFinalState.SUCCESS);
+        operationTracker.onResponse(chunkPutRequestInfo.replicaId, TrackedRequestFinalState.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, chunkPutRequestInfo.replicaId)) {
           logger.trace("Cross colo request successful for remote replica in {} ",
               chunkPutRequestInfo.replicaId.getDataNodeId().getDatacenterName());
@@ -1424,7 +1424,7 @@ void handleResponse(ResponseInfo responseInfo, PutResponse putResponse) {
      * @param replicaId the {@link ReplicaId} associated with the failed response.
      */
     void onErrorResponse(ReplicaId replicaId) {
-      operationTracker.onResponse(replicaId, ServerRequestFinalState.FAILURE);
+      operationTracker.onResponse(replicaId, TrackedRequestFinalState.FAILURE);
       routerMetrics.routerRequestErrorCount.inc();
       routerMetrics.getDataNodeBasedMetrics(replicaId.getDataNodeId()).putRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
index 859874c2ff..2d06f62c01 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/SimpleOperationTracker.java
@@ -185,9 +185,9 @@ public boolean isDone() {
   }
 
   @Override
-  public void onResponse(ReplicaId replicaId, ServerRequestFinalState serverRequestFinalState) {
+  public void onResponse(ReplicaId replicaId, TrackedRequestFinalState trackedRequestFinalState) {
     inflightCount--;
-    if (serverRequestFinalState == ServerRequestFinalState.SUCCESS) {
+    if (trackedRequestFinalState == TrackedRequestFinalState.SUCCESS) {
       succeededCount++;
     } else {
       failedCount++;
diff --git a/ambry-router/src/main/java/com.github.ambry.router/ServerRequestFinalState.java b/ambry-router/src/main/java/com.github.ambry.router/TrackedRequestFinalState.java
similarity index 59%
rename from ambry-router/src/main/java/com.github.ambry.router/ServerRequestFinalState.java
rename to ambry-router/src/main/java/com.github.ambry.router/TrackedRequestFinalState.java
index 66e83f331d..a1b6ceac1a 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/ServerRequestFinalState.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/TrackedRequestFinalState.java
@@ -14,8 +14,10 @@
 package com.github.ambry.router;
 
 /**
- * The final state of a single request created to be sent to server.
+ * The final state of a single request that is tracked by operation tracker. Note that, after request is created in router,
+ * it is not guaranteed to be sent out. The request may already time out during connection checkout etc. This enum is
+ * consumed by operation tracker to change success/failure counter and determine whether to update Histograms.
  */
-public enum ServerRequestFinalState {
+public enum TrackedRequestFinalState {
   SUCCESS, FAILURE, TIMED_OUT
 }
diff --git a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
index 917ac06d2f..9641048c07 100644
--- a/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
+++ b/ambry-router/src/main/java/com.github.ambry.router/TtlUpdateOperation.java
@@ -243,7 +243,7 @@ private void processServerError(ReplicaId replica, ServerErrorCode serverErrorCo
     switch (serverErrorCode) {
       case No_Error:
       case Blob_Already_Updated:
-        operationTracker.onResponse(replica, ServerRequestFinalState.SUCCESS);
+        operationTracker.onResponse(replica, TrackedRequestFinalState.SUCCESS);
         if (RouterUtils.isRemoteReplica(routerConfig, replica)) {
           LOGGER.trace("Cross colo request successful for remote replica {} in {} ", replica.getDataNodeId(),
               replica.getDataNodeId().getDatacenterName());
@@ -298,8 +298,8 @@ private void updateOperationState(ReplicaId replica, RouterErrorCode error) {
       }
     }
     operationTracker.onResponse(replica,
-        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? ServerRequestFinalState.TIMED_OUT
-            : ServerRequestFinalState.FAILURE);
+        resolvedRouterErrorCode == RouterErrorCode.OperationTimedOut ? TrackedRequestFinalState.TIMED_OUT
+            : TrackedRequestFinalState.FAILURE);
     if (error != RouterErrorCode.BlobDeleted && error != RouterErrorCode.BlobExpired) {
       routerMetrics.routerRequestErrorCount.inc();
     }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
index 70da82e468..c7a40e9927 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/AdaptiveOperationTrackerTest.java
@@ -144,7 +144,7 @@ public void adaptationTest() throws InterruptedException {
     // generate a response for every request and make sure there are no errors
     for (int i = 0; i < REPLICA_COUNT; i++) {
       assertFalse("Operation should not be done", ot.isDone());
-      ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
+      ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.SUCCESS);
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be REPLICA_COUNT - 2
@@ -191,13 +191,13 @@ public void noUnexpiredRequestsTest() throws InterruptedException {
     sendRequests(ot, 1);
     // 1-2-0-0
     // provide a response to the second request that is not a success
-    ot.onResponse(inflightReplicas.pollLast(), ServerRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.pollLast(), TrackedRequestFinalState.FAILURE);
     // 1-1-0-1
     assertFalse("Operation should not be done", ot.isDone());
     // should now be able to send one more request
     sendRequests(ot, 1);
     // 0-2-0-1
-    ot.onResponse(inflightReplicas.pollLast(), ServerRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.pollLast(), TrackedRequestFinalState.SUCCESS);
     // 0-1-1-1
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -235,7 +235,7 @@ public void trackerUpdateBetweenHasNextAndNextTest() throws InterruptedException
 
     sendRequests(ot, 1);
     // 1-2-0-0
-    ot.onResponse(inflightReplicas.pollLast(), ServerRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.pollLast(), TrackedRequestFinalState.SUCCESS);
     // 1-1-1-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     // past due counter should be 1
@@ -393,7 +393,7 @@ private void verifyHistogramRecording(OperationTracker ot, boolean succeedReques
     for (double expectedAverage : expectedAverages) {
       time.sleep(timeIncrement);
       ot.onResponse(inflightReplicas.poll(),
-          succeedRequests ? ServerRequestFinalState.SUCCESS : ServerRequestFinalState.FAILURE);
+          succeedRequests ? TrackedRequestFinalState.SUCCESS : TrackedRequestFinalState.FAILURE);
       assertEquals("Average does not match. Histogram recording may be incorrect", expectedAverage,
           tracker.getSnapshot().getMean(), 0.001);
     }
diff --git a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
index 12a18a4866..f2162a5e31 100644
--- a/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
+++ b/ambry-router/src/test/java/com.github.ambry.router/OperationTrackerTest.java
@@ -117,13 +117,13 @@ public void localSucceedTest() {
     // 0-3-0-0; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
+      ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.SUCCESS);
     }
     // 0-1-2-0; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
 
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.FAILURE);
     // 0-0-2-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -146,12 +146,12 @@ public void localFailTest() {
     sendRequests(ot, 3, false);
     // 0-3-0-0; 9-0-0-0
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
+      ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.FAILURE);
     }
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
     // 0-1-0-2; 9-0-0-0
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.SUCCESS);
     // 0-0-1-2; 9-0-0-0
     assertFalse("Operation should not have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -174,14 +174,14 @@ public void localSucceedWithDifferentParameterTest() {
     sendRequests(ot, 2, false);
     // 1-2-0-0; 9-0-0-0
 
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.FAILURE);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not have been done.", ot.isDone());
 
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.SUCCESS);
     // 0-1-1-1; 9-0-0-0
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
@@ -212,35 +212,35 @@ public void remoteReplicaTest() {
     // 1-2-0-0; 9-0-0-0
     ReplicaId id = inflightReplicas.poll();
     assertEquals("First request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, ServerRequestFinalState.FAILURE);
+    ot.onResponse(id, TrackedRequestFinalState.FAILURE);
     // 1-1-0-1; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
     // 0-2-0-1; 9-0-0-0
     id = inflightReplicas.poll();
     assertEquals("Second request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, ServerRequestFinalState.FAILURE);
+    ot.onResponse(id, TrackedRequestFinalState.FAILURE);
     id = inflightReplicas.poll();
     assertEquals("Third request should have been to local DC", localDcName, id.getDataNodeId().getDatacenterName());
-    ot.onResponse(id, ServerRequestFinalState.FAILURE);
+    ot.onResponse(id, TrackedRequestFinalState.FAILURE);
     // 0-0-0-3; 9-0-0-0
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 7-2-0-0
     assertFalse("Operation should not be done", ot.isDone());
     for (int i = 0; i < 2; i++) {
-      ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
+      ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.FAILURE);
     }
     // 0-0-0-3; 7-0-0-2
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 2, false);
     // 0-0-0-3; 5-2-0-2
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     // 0-0-0-3; 5-1-0-3
     sendRequests(ot, 1, false);
     // 0-0-0-3; 4-1-0-3
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.SUCCESS);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -262,7 +262,7 @@ public void fullSuccessTargetTest() {
     while (!ot.hasSucceeded()) {
       sendRequests(ot, 3, false);
       for (int i = 0; i < 3; i++) {
-        ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
+        ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.SUCCESS);
       }
     }
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -295,14 +295,14 @@ public void useReplicaNotSucceededSendTest() {
     localDcName = datanodes.get(0).getDatacenterName();
     OperationTracker ot = getOperationTracker(true, 1, 2, true, Integer.MAX_VALUE);
     sendRequests(ot, 2, true);
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, true);
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.FAILURE);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.FAILURE);
     assertFalse("Operation should not be done", ot.isDone());
     sendRequests(ot, 1, false);
-    ot.onResponse(inflightReplicas.poll(), ServerRequestFinalState.SUCCESS);
+    ot.onResponse(inflightReplicas.poll(), TrackedRequestFinalState.SUCCESS);
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
     assertTrue("Operation should be done", ot.isDone());
   }
@@ -338,12 +338,12 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, ServerRequestFinalState.FAILURE);
+      ot.onResponse(replica, TrackedRequestFinalState.FAILURE);
     }
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, ServerRequestFinalState.FAILURE);
+      ot.onResponse(replica, TrackedRequestFinalState.FAILURE);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
@@ -351,7 +351,7 @@ public void replicasOrderingTestOriginatingUnknown() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, ServerRequestFinalState.SUCCESS);
+      ot.onResponse(replica, TrackedRequestFinalState.SUCCESS);
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
     assertTrue("Operation should have succeeded", ot.hasSucceeded());
@@ -369,7 +369,7 @@ public void replicasOrderingTestOriginatingIsLocal() {
     sendRequests(ot, 3, false);
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, ServerRequestFinalState.SUCCESS);
+      ot.onResponse(replica, TrackedRequestFinalState.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -389,14 +389,14 @@ public void replicasOrderingTestOriginatingNotLocal() {
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
       // fail first 3 requests to local
-      ot.onResponse(replica, ServerRequestFinalState.FAILURE);
+      ot.onResponse(replica, TrackedRequestFinalState.FAILURE);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, ServerRequestFinalState.SUCCESS);
+      ot.onResponse(replica, TrackedRequestFinalState.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());
@@ -418,14 +418,14 @@ public void replicasOrderTestOriginatingDcOnly() {
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
       // fail first 3 requests to local replicas
-      ot.onResponse(replica, ServerRequestFinalState.FAILURE);
+      ot.onResponse(replica, TrackedRequestFinalState.FAILURE);
       assertEquals("Should be local DC", localDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertFalse("Operation should have not succeeded", ot.hasSucceeded());
 
     for (int i = 0; i < 3; i++) {
       ReplicaId replica = inflightReplicas.poll();
-      ot.onResponse(replica, ServerRequestFinalState.SUCCESS);
+      ot.onResponse(replica, TrackedRequestFinalState.SUCCESS);
       assertEquals("Should be originating DC", originatingDcName, replica.getDataNodeId().getDatacenterName());
     }
     assertEquals("Should have 0 replica in flight.", 0, inflightReplicas.size());