loadbalancer: Add the ErrorClass enum and use it in RequestTracker (#…

…2808)

Motivation:

RequestTracker has three observer methods: `onSuccess`, `onCancel`,
and `onError`. onCancel is really a special case of `onError` but it
can be helpful to distinguish them at times which leads to the more
general problem: it is often helpful to know what kind of error
it was so we can establish the origin.

Modifications:

- Add the `ErrorClass` enum for classifying `RequestTracker` errors.
- Consolidate the `onCancel` method into the `onError` method by
  having a `CANCELLED` variant of the `ErrorClass` enum.

Result:

Less methods and more information about the origin of errors.

servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/DefaultRequestTracker.java

@@ -93,15 +93,10 @@ public void onSuccess(final long startTimeNanos) {
     }
 
     @Override
-    public void onCancel(final long startTimeNanos) {
+    public void onError(final long startTimeNanos, ErrorClass errorClass) {
         pendingUpdater.decrementAndGet(this);
-        calculateAndStore(this::cancelPenalty, startTimeNanos);
-    }
-
-    @Override
-    public void onError(final long startTimeNanos) {
-        pendingUpdater.decrementAndGet(this);
-        calculateAndStore(this::errorPenalty, startTimeNanos);
+        calculateAndStore(errorClass == ErrorClass.CANCELLED ? this:: cancelPenalty : this::errorPenalty,
+                startTimeNanos);
     }
 
     @Override

servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/ErrorClass.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright © 2024 Apple Inc. and the ServiceTalk project authors
+ *
+ * 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.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.servicetalk.loadbalancer;
+
+/**
+ * Enumeration of the main failure classes.
+ */
+enum ErrorClass {
+
+    /**
+     * Failures related to locally enforced timeouts that prevent session establishment with the peer.
+     */
+    LOCAL_ORIGIN_TIMEOUT(true),
+    /**
+     * Failures related to connection establishment.
+     */
+    LOCAL_ORIGIN_CONNECT_FAILED(true),
+
+    /**
+     * Failures related to locally enforced timeouts waiting for responses from the peer.
+     */
+    EXT_ORIGIN_TIMEOUT(false),
+
+    /**
+     * Failures returned from the remote peer. This will be things like 5xx responses.
+     */
+    EXT_ORIGIN_REQUEST_FAILED(false),
+
+    /**
+     * Failure due to cancellation.
+     */
+    CANCELLED(true);
+
+    private final boolean isLocal;
+    ErrorClass(boolean isLocal) {
+        this.isLocal = isLocal;
+    }
+
+    public boolean isLocal() {
+        return isLocal;
+    }
+}

servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RequestTracker.java

@@ -19,6 +19,16 @@
 
 /**
  * A tracker of latency of an action over time.
+ * <p>
+ * The usage of the RequestTracker is intended to follow the simple workflow:
+ * - At initiation of an action for which a request is must call {@link RequestTracker#beforeStart()} and save the
+ *   timestamp much like would be done when using a stamped lock.
+ * - Once the request event is complete only one of the {@link RequestTracker#onSuccess(long)} or
+ *   {@link RequestTracker#onError(long, ErrorClass, Throwable)} methods must be called and called exactly once.
+ * In other words, every call to {@link RequestTracker#beforeStart()} must be followed by exactly one call to either of
+ * the completion methods {@link RequestTracker#onSuccess(long)} or
+ * {@link RequestTracker#onError(long, ErrorClass, Throwable)}. Failure to do so can cause state corruption in the
+ * {@link RequestTracker} implementations which may track not just latency but also the outstanding requests.
  */
 interface RequestTracker {
 
@@ -39,17 +49,11 @@ interface RequestTracker {
      */
     void onSuccess(long beforeStartTimeNs);
 
-    /**
-     * Records cancellation of the action for which latency is to be tracked.
-     *
-     * @param beforeStartTimeNs return value from {@link #beforeStart()}.
-     */
-    void onCancel(long beforeStartTimeNs);
-
     /**
      * Records a failed completion of the action for which latency is to be tracked.
      *
      * @param beforeStartTimeNs return value from {@link #beforeStart()}.
+     * @param errorClass the class of error that triggered this method.
      */
-    void onError(long beforeStartTimeNs);
+    void onError(long beforeStartTimeNs, ErrorClass errorClass);
 }

servicetalk-loadbalancer/src/main/java/io/servicetalk/loadbalancer/RootSwayingLeafRequestTracker.java

@@ -47,16 +47,9 @@ public void onSuccess(final long beforeStartTimeNs) {
     }
 
     @Override
-    public void onCancel(final long beforeStartTimeNs) {
+    public void onError(final long beforeStartTimeNs, ErrorClass errorClass) {
         // Tracks both levels
-        root.onCancel(beforeStartTimeNs);
-        leaf.onCancel(beforeStartTimeNs);
-    }
-
-    @Override
-    public void onError(final long beforeStartTimeNs) {
-        // Tracks both levels
-        root.onError(beforeStartTimeNs);
-        leaf.onError(beforeStartTimeNs);
+        root.onError(beforeStartTimeNs, errorClass);
+        leaf.onError(beforeStartTimeNs, errorClass);
     }
 }

servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/DefaultLoadBalancerTest.java

@@ -214,11 +214,7 @@ public void onSuccess(long beforeStartTime) {
         }
 
         @Override
-        public void onCancel(long beforeStartTimeNs) {
-        }
-
-        @Override
-        public void onError(long beforeStartTime) {
+        public void onError(long beforeStartTime, ErrorClass errorClass) {
         }
     }
 

servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/DefaultRequestTrackerTest.java

@@ -41,9 +41,13 @@ void test() {
         Assertions.assertEquals(-500, requestTracker.score());
 
         // error penalty
-        requestTracker.onError(requestTracker.beforeStart());
+        requestTracker.onError(requestTracker.beforeStart(), ErrorClass.LOCAL_ORIGIN_CONNECT_FAILED);
         Assertions.assertEquals(-5000, requestTracker.score());
 
+        // cancellation penalty
+        requestTracker.onError(requestTracker.beforeStart(), ErrorClass.CANCELLED);
+        Assertions.assertEquals(-12_500, requestTracker.score());
+
         // decay
         when(nextValueProvider.applyAsLong(anyLong())).thenAnswer(__ -> ofSeconds(20).toNanos());
         Assertions.assertEquals(-1, requestTracker.score());