Limit calls to Conversions.resolveExecutionProfile

chibenwa · absurdfarce · commit d0a1e44a4415 · 2024-07-02T10:53:22.000-05:00
Those repeated calls account for a non-negligible portion of my application CPU (0.6%) and can definitly be a final field so that it gets resolved only once per CqlRequestHandler. patch by Benoit Tellier; reviewed by Andy Tolbert, and Bret McGuire reference: #1623
diff --git a/core/src/main/java/com/datastax/dse/driver/internal/core/cql/continuous/ContinuousRequestHandlerBase.java b/core/src/main/java/com/datastax/dse/driver/internal/core/cql/continuous/ContinuousRequestHandlerBase.java
@@ -648,12 +648,13 @@ public void operationComplete(@NonNull Future<java.lang.Void> future) {
         }
       } else {
         LOG.trace("[{}] Request sent on {}", logPrefix, channel);
-        if (scheduleSpeculativeExecution && Conversions.resolveIdempotence(statement, context)) {
+        if (scheduleSpeculativeExecution
+            && Conversions.resolveIdempotence(statement, executionProfile)) {
           int nextExecution = executionIndex + 1;
           // Note that `node` is the first node of the execution, it might not be the "slow" one
           // if there were retries, but in practice retries are rare.
           long nextDelay =
-              Conversions.resolveSpeculativeExecutionPolicy(statement, context)
+              Conversions.resolveSpeculativeExecutionPolicy(context, executionProfile)
                   .nextExecution(node, keyspace, statement, nextExecution);
           if (nextDelay >= 0) {
             scheduleSpeculativeExecution(nextExecution, nextDelay);
@@ -787,12 +788,12 @@ public void onFailure(@NonNull Throwable error) {
       cancelTimeout(pageTimeout);
       LOG.trace(String.format("[%s] Request failure", logPrefix), error);
       RetryVerdict verdict;
-      if (!Conversions.resolveIdempotence(statement, context)
+      if (!Conversions.resolveIdempotence(statement, executionProfile)
           || error instanceof FrameTooLongException) {
         verdict = RetryVerdict.RETHROW;
       } else {
         try {
-          RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(statement, context);
+          RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(context, executionProfile);
           verdict = retryPolicy.onRequestAbortedVerdict(statement, error, retryCount);
         } catch (Throwable cause) {
           abort(
@@ -945,7 +946,7 @@ private void processRecoverableError(@NonNull CoordinatorException error) {
       assert lock.isHeldByCurrentThread();
       NodeMetricUpdater metricUpdater = ((DefaultNode) node).getMetricUpdater();
       RetryVerdict verdict;
-      RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(statement, context);
+      RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(context, executionProfile);
       if (error instanceof ReadTimeoutException) {
         ReadTimeoutException readTimeout = (ReadTimeoutException) error;
         verdict =
@@ -964,7 +965,7 @@ private void processRecoverableError(@NonNull CoordinatorException error) {
             DefaultNodeMetric.IGNORES_ON_READ_TIMEOUT);
       } else if (error instanceof WriteTimeoutException) {
         WriteTimeoutException writeTimeout = (WriteTimeoutException) error;
-        if (Conversions.resolveIdempotence(statement, context)) {
+        if (Conversions.resolveIdempotence(statement, executionProfile)) {
           verdict =
               retryPolicy.onWriteTimeoutVerdict(
                   statement,
@@ -999,7 +1000,7 @@ private void processRecoverableError(@NonNull CoordinatorException error) {
             DefaultNodeMetric.IGNORES_ON_UNAVAILABLE);
       } else {
         verdict =
-            Conversions.resolveIdempotence(statement, context)
+            Conversions.resolveIdempotence(statement, executionProfile)
                 ? retryPolicy.onErrorResponseVerdict(statement, error, retryCount)
                 : RetryVerdict.RETHROW;
         updateErrorMetrics(
diff --git a/core/src/main/java/com/datastax/dse/driver/internal/core/graph/GraphRequestHandler.java b/core/src/main/java/com/datastax/dse/driver/internal/core/graph/GraphRequestHandler.java
@@ -557,12 +557,13 @@ public void operationComplete(Future<java.lang.Void> future) {
           cancel();
         } else {
           inFlightCallbacks.add(this);
-          if (scheduleNextExecution && Conversions.resolveIdempotence(statement, context)) {
+          if (scheduleNextExecution
+              && Conversions.resolveIdempotence(statement, executionProfile)) {
             int nextExecution = execution + 1;
             long nextDelay;
             try {
               nextDelay =
-                  Conversions.resolveSpeculativeExecutionPolicy(statement, context)
+                  Conversions.resolveSpeculativeExecutionPolicy(context, executionProfile)
                       .nextExecution(node, null, statement, nextExecution);
             } catch (Throwable cause) {
               // This is a bug in the policy, but not fatal since we have at least one other
@@ -678,7 +679,7 @@ private void processErrorResponse(Error errorMessage) {
         trackNodeError(node, error, NANOTIME_NOT_MEASURED_YET);
         setFinalError(statement, error, node, execution);
       } else {
-        RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(statement, context);
+        RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(context, executionProfile);
         RetryVerdict verdict;
         if (error instanceof ReadTimeoutException) {
           ReadTimeoutException readTimeout = (ReadTimeoutException) error;
@@ -699,7 +700,7 @@ private void processErrorResponse(Error errorMessage) {
         } else if (error instanceof WriteTimeoutException) {
           WriteTimeoutException writeTimeout = (WriteTimeoutException) error;
           verdict =
-              Conversions.resolveIdempotence(statement, context)
+              Conversions.resolveIdempotence(statement, executionProfile)
                   ? retryPolicy.onWriteTimeoutVerdict(
                       statement,
                       writeTimeout.getConsistencyLevel(),
@@ -731,7 +732,7 @@ private void processErrorResponse(Error errorMessage) {
               DefaultNodeMetric.IGNORES_ON_UNAVAILABLE);
         } else {
           verdict =
-              Conversions.resolveIdempotence(statement, context)
+              Conversions.resolveIdempotence(statement, executionProfile)
                   ? retryPolicy.onErrorResponseVerdict(statement, error, retryCount)
                   : RetryVerdict.RETHROW;
           updateErrorMetrics(
@@ -810,12 +811,12 @@ public void onFailure(Throwable error) {
       }
       LOG.trace("[{}] Request failure, processing: {}", logPrefix, error);
       RetryVerdict verdict;
-      if (!Conversions.resolveIdempotence(statement, context)
+      if (!Conversions.resolveIdempotence(statement, executionProfile)
           || error instanceof FrameTooLongException) {
         verdict = RetryVerdict.RETHROW;
       } else {
         try {
-          RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(statement, context);
+          RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(context, executionProfile);
           verdict = retryPolicy.onRequestAbortedVerdict(statement, error, retryCount);
         } catch (Throwable cause) {
           setFinalError(
diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/cql/Conversions.java b/core/src/main/java/com/datastax/oss/driver/internal/core/cql/Conversions.java
@@ -535,29 +535,59 @@ public static CoordinatorException toThrowable(
     }
   }
 
+  /** Use {@link #resolveIdempotence(Request, DriverExecutionProfile)} instead. */
+  @Deprecated
   public static boolean resolveIdempotence(Request request, InternalDriverContext context) {
+    return resolveIdempotence(request, resolveExecutionProfile(request, context));
+  }
+
+  public static boolean resolveIdempotence(
+      Request request, DriverExecutionProfile executionProfile) {
     Boolean requestIsIdempotent = request.isIdempotent();
-    DriverExecutionProfile executionProfile = resolveExecutionProfile(request, context);
     return (requestIsIdempotent == null)
         ? executionProfile.getBoolean(DefaultDriverOption.REQUEST_DEFAULT_IDEMPOTENCE)
         : requestIsIdempotent;
   }
 
+  /** Use {@link #resolveRequestTimeout(Request, DriverExecutionProfile)} instead. */
+  @Deprecated
   public static Duration resolveRequestTimeout(Request request, InternalDriverContext context) {
-    DriverExecutionProfile executionProfile = resolveExecutionProfile(request, context);
-    return request.getTimeout() != null
-        ? request.getTimeout()
+    return resolveRequestTimeout(request, resolveExecutionProfile(request, context));
+  }
+
+  public static Duration resolveRequestTimeout(
+      Request request, DriverExecutionProfile executionProfile) {
+    Duration timeout = request.getTimeout();
+    return timeout != null
+        ? timeout
         : executionProfile.getDuration(DefaultDriverOption.REQUEST_TIMEOUT);
   }
 
+  /** Use {@link #resolveRetryPolicy(InternalDriverContext, DriverExecutionProfile)} instead. */
+  @Deprecated
   public static RetryPolicy resolveRetryPolicy(Request request, InternalDriverContext context) {
     DriverExecutionProfile executionProfile = resolveExecutionProfile(request, context);
     return context.getRetryPolicy(executionProfile.getName());
   }
 
+  public static RetryPolicy resolveRetryPolicy(
+      InternalDriverContext context, DriverExecutionProfile executionProfile) {
+    return context.getRetryPolicy(executionProfile.getName());
+  }
+
+  /**
+   * Use {@link #resolveSpeculativeExecutionPolicy(InternalDriverContext, DriverExecutionProfile)}
+   * instead.
+   */
+  @Deprecated
   public static SpeculativeExecutionPolicy resolveSpeculativeExecutionPolicy(
       Request request, InternalDriverContext context) {
     DriverExecutionProfile executionProfile = resolveExecutionProfile(request, context);
     return context.getSpeculativeExecutionPolicy(executionProfile.getName());
   }
+
+  public static SpeculativeExecutionPolicy resolveSpeculativeExecutionPolicy(
+      InternalDriverContext context, DriverExecutionProfile executionProfile) {
+    return context.getSpeculativeExecutionPolicy(executionProfile.getName());
+  }
 }
diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlPrepareHandler.java b/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlPrepareHandler.java
@@ -92,6 +92,7 @@ public class CqlPrepareHandler implements Throttled {
   private final Timeout scheduledTimeout;
   private final RequestThrottler throttler;
   private final Boolean prepareOnAllNodes;
+  private final DriverExecutionProfile executionProfile;
   private volatile InitialPrepareCallback initialCallback;
 
   // The errors on the nodes that were already tried (lazily initialized on the first error).
@@ -111,7 +112,7 @@ protected CqlPrepareHandler(
     this.initialRequest = request;
     this.session = session;
     this.context = context;
-    DriverExecutionProfile executionProfile = Conversions.resolveExecutionProfile(request, context);
+    executionProfile = Conversions.resolveExecutionProfile(request, context);
     this.queryPlan =
         context
             .getLoadBalancingPolicyWrapper()
@@ -131,7 +132,7 @@ protected CqlPrepareHandler(
         });
     this.timer = context.getNettyOptions().getTimer();
 
-    Duration timeout = Conversions.resolveRequestTimeout(request, context);
+    Duration timeout = Conversions.resolveRequestTimeout(request, executionProfile);
     this.scheduledTimeout = scheduleTimeout(timeout);
     this.prepareOnAllNodes = executionProfile.getBoolean(DefaultDriverOption.PREPARE_ON_ALL_NODES);
 
@@ -292,7 +293,7 @@ private CompletionStage<Void> prepareOnOtherNode(PrepareRequest request, Node no
               false,
               toPrepareMessage(request),
               request.getCustomPayload(),
-              Conversions.resolveRequestTimeout(request, context),
+              Conversions.resolveRequestTimeout(request, executionProfile),
               throttler,
               session.getMetricUpdater(),
               logPrefix);
@@ -419,7 +420,7 @@ private void processErrorResponse(Error errorMessage) {
       } else {
         // Because prepare requests are known to always be idempotent, we call the retry policy
         // directly, without checking the flag.
-        RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(request, context);
+        RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(context, executionProfile);
         RetryVerdict verdict = retryPolicy.onErrorResponseVerdict(request, error, retryCount);
         processRetryVerdict(verdict, error);
       }
@@ -457,7 +458,7 @@ public void onFailure(Throwable error) {
       LOG.trace("[{}] Request failure, processing: {}", logPrefix, error.toString());
       RetryVerdict verdict;
       try {
-        RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(request, context);
+        RetryPolicy retryPolicy = Conversions.resolveRetryPolicy(context, executionProfile);
         verdict = retryPolicy.onRequestAbortedVerdict(request, error, retryCount);
       } catch (Throwable cause) {
         setFinalError(
diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java b/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java
