Avoid counting async dispatches in request count stats

Fixme cleanup. Remove fragile instanceof logic.
markt-asf · Feb 22, 2024 · f63f144 · f63f144
1 parent 7f9af2f
commit f63f144
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 24 deletions.
diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -62,6 +62,8 @@ public class DeltaManager extends ClusterManagerBase {
      */
     protected static final StringManager sm = StringManager.getManager(DeltaManager.class);
 
+    private static final String[] EMPTY_STRING_ARRAY = new String[0];
+
     // ----------------------------------------------------- Instance Variables
 
     protected String name = null;
@@ -1052,7 +1054,7 @@ public void expireAllLocalSessions() {
 
     @Override
     public String[] getInvalidatedSessions() {
-        return new String[0];
+        return EMPTY_STRING_ARRAY;
     }
 
     // -------------------------------------------------------- message receive

diff --git a/java/org/apache/catalina/ha/tcp/ReplicationValve.java b/java/org/apache/catalina/ha/tcp/ReplicationValve.java
@@ -306,12 +306,12 @@ public void invoke(Request request, Response response) throws IOException, Servl
         }
         Context context = request.getContext();
         boolean isCrossContext = context != null && context instanceof StandardContext && context.getCrossContext();
+        boolean isAsync = request.getAsyncContextInternal() != null;
         try {
             if (isCrossContext) {
                 if (log.isTraceEnabled()) {
                     log.trace(sm.getString("ReplicationValve.crossContext.add"));
                 }
-                // FIXME add Pool of Arraylists
                 crossContextSessions.set(new ArrayList<>());
             }
             getNext().invoke(request, response);
@@ -324,7 +324,7 @@ public void invoke(Request request, Response response) throws IOException, Servl
                     return;
                 }
                 if (cluster.hasMembers()) {
-                    sendReplicationMessage(request, totalstart, isCrossContext, clusterManager);
+                    sendReplicationMessage(request, totalstart, isCrossContext, isAsync, clusterManager);
                 } else {
                     resetReplicationRequest(request, isCrossContext);
                 }
@@ -380,7 +380,7 @@ protected void startInternal() throws LifecycleException {
 
     // --------------------------------------------------------- Protected Methods
 
-    protected void sendReplicationMessage(Request request, long totalstart, boolean isCrossContext,
+    protected void sendReplicationMessage(Request request, long totalstart, boolean isCrossContext, boolean isAsync,
             ClusterManager clusterManager) {
         // this happens after the request
         long start = 0;
@@ -389,10 +389,7 @@ protected void sendReplicationMessage(Request request, long totalstart, boolean
         }
         try {
             // send invalid sessions
-            // DeltaManager returns String[0]
-            if (!(clusterManager instanceof DeltaManager)) {
-                sendInvalidSessions(clusterManager);
-            }
+            sendInvalidSessions(clusterManager);
             // send replication
             sendSessionReplicationMessage(request, clusterManager);
             if (isCrossContext) {
@@ -403,7 +400,7 @@ protected void sendReplicationMessage(Request request, long totalstart, boolean
             log.error(sm.getString("ReplicationValve.send.failure"), x);
         } finally {
             if (doStatistics()) {
-                updateStats(totalstart, start);
+                updateStats(totalstart, start, isAsync);
             }
         }
     }
@@ -415,8 +412,8 @@ protected void sendCrossContextSession() {
         List<DeltaSession> sessions = crossContextSessions.get();
         if (sessions != null && sessions.size() > 0) {
             for (DeltaSession session : sessions) {
-                if (log.isDebugEnabled()) {
-                    log.debug(sm.getString("ReplicationValve.crossContext.sendDelta",
+                if (log.isTraceEnabled()) {
+                    log.trace(sm.getString("ReplicationValve.crossContext.sendDelta",
                             session.getManager().getContext().getName()));
                 }
                 sendMessage(session, (ClusterManager) session.getManager());
@@ -557,23 +554,24 @@ protected boolean isRequestWithoutSessionChange(String uri) {
      *
      * @param requestTime Request time
      * @param clusterTime Cluster time
+     * @param isAsync if the request was in async mode
      */
-    protected void updateStats(long requestTime, long clusterTime) {
-        // TODO: Async requests may trigger multiple replication requests. How,
-        // if at all, should the stats handle this?
+    protected void updateStats(long requestTime, long clusterTime, boolean isAsync) {
         long currentTime = System.currentTimeMillis();
         lastSendTime.set(currentTime);
         totalSendTime.add(currentTime - clusterTime);
         totalRequestTime.add(currentTime - requestTime);
-        nrOfRequests.increment();
-        if (log.isInfoEnabled()) {
-            if ((nrOfRequests.longValue() % 100) == 0) {
-                log.info(sm.getString("ReplicationValve.stats",
-                        new Object[] { Long.valueOf(totalRequestTime.longValue() / nrOfRequests.longValue()),
-                                Long.valueOf(totalSendTime.longValue() / nrOfRequests.longValue()), Long.valueOf(nrOfRequests.longValue()),
-                                Long.valueOf(nrOfSendRequests.longValue()), Long.valueOf(nrOfCrossContextSendRequests.longValue()),
-                                Long.valueOf(nrOfFilterRequests.longValue()), Long.valueOf(totalRequestTime.longValue()),
-                                Long.valueOf(totalSendTime.longValue()) }));
+        if (!isAsync) {
+            nrOfRequests.increment();
+            if (log.isDebugEnabled()) {
+                if ((nrOfRequests.longValue() % 100) == 0) {
+                    log.debug(sm.getString("ReplicationValve.stats",
+                            new Object[] { Long.valueOf(totalRequestTime.longValue() / nrOfRequests.longValue()),
+                                    Long.valueOf(totalSendTime.longValue() / nrOfRequests.longValue()), Long.valueOf(nrOfRequests.longValue()),
+                                    Long.valueOf(nrOfSendRequests.longValue()), Long.valueOf(nrOfCrossContextSendRequests.longValue()),
+                                    Long.valueOf(nrOfFilterRequests.longValue()), Long.valueOf(totalRequestTime.longValue()),
+                                    Long.valueOf(totalSendTime.longValue()) }));
+                }
             }
         }
     }

diff --git a/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java b/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
@@ -58,7 +58,7 @@
 
 /**
  * A <b>Cluster </b> implementation using simple multicast. Responsible for setting up a cluster and provides callers
- * with a valid multicast receiver/sender. FIXME wrote testcases
+ * with a valid multicast receiver/sender.
  *
  * @author Remy Maucherat
  * @author Peter Rossbach

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -128,6 +128,13 @@
       </update>
     </changelog>
   </subsection>
+  <subsection name="Cluster">
+    <changelog>
+      <fix>
+        Avoid updating request count stats on async. (remm)
+      </fix>
+    </changelog>
+  </subsection>
 </section>
 <section name="Tomcat 11.0.0-M17 (markt)" rtext="2024-02-19">
   <subsection name="Catalina">