This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push: new f63f14419f Avoid counting async dispatches in request count stats f63f14419f is described below commit f63f14419f128886f0094243282537f87a551381 Author: remm <r...@apache.org> AuthorDate: Thu Feb 22 14:10:06 2024 +0100 Avoid counting async dispatches in request count stats Fixme cleanup. Remove fragile instanceof logic. --- .../apache/catalina/ha/session/DeltaManager.java | 4 ++- .../apache/catalina/ha/tcp/ReplicationValve.java | 42 +++++++++++----------- .../apache/catalina/ha/tcp/SimpleTcpCluster.java | 2 +- webapps/docs/changelog.xml | 7 ++++ 4 files changed, 31 insertions(+), 24 deletions(-) diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java index 739107e1f9..2bb7439986 100644 --- 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 class DeltaManager extends ClusterManagerBase { @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 index de419319ae..4470d50e89 100644 --- a/java/org/apache/catalina/ha/tcp/ReplicationValve.java +++ b/java/org/apache/catalina/ha/tcp/ReplicationValve.java @@ -306,12 +306,12 @@ public class ReplicationValve extends ValveBase implements ClusterValve { } 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 class ReplicationValve extends ValveBase implements ClusterValve { return; } if (cluster.hasMembers()) { - sendReplicationMessage(request, totalstart, isCrossContext, clusterManager); + sendReplicationMessage(request, totalstart, isCrossContext, isAsync, clusterManager); } else { resetReplicationRequest(request, isCrossContext); } @@ -380,7 +380,7 @@ public class ReplicationValve extends ValveBase implements ClusterValve { // --------------------------------------------------------- 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 @@ public class ReplicationValve extends ValveBase implements ClusterValve { } 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 @@ public class ReplicationValve extends ValveBase implements ClusterValve { log.error(sm.getString("ReplicationValve.send.failure"), x); } finally { if (doStatistics()) { - updateStats(totalstart, start); + updateStats(totalstart, start, isAsync); } } } @@ -415,8 +412,8 @@ public class ReplicationValve extends ValveBase implements ClusterValve { 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 @@ public class ReplicationValve extends ValveBase implements ClusterValve { * * @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 index c0d3cad81c..7e787dc956 100644 --- a/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java +++ b/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java @@ -58,7 +58,7 @@ import org.apache.tomcat.util.res.StringManager; /** * 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 index 5f3a32e564..241ac0f4bc 100644 --- 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"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org