Author: markt Date: Fri Feb 23 14:43:40 2018 New Revision: 1825130 URL: http://svn.apache.org/viewvc?rev=1825130&view=rev Log: Fix a SpotBugs report Switch to volatile for stats to avoid corruption from concurrent updates Refactor to remove need for existing sync Document that some updates maybe lost (this was the case before the change)
Modified: tomcat/trunk/java/org/apache/catalina/ha/tcp/ReplicationValve.java tomcat/trunk/res/findbugs/filter-false-positives.xml Modified: tomcat/trunk/java/org/apache/catalina/ha/tcp/ReplicationValve.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/tcp/ReplicationValve.java?rev=1825130&r1=1825129&r2=1825130&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/ha/tcp/ReplicationValve.java (original) +++ tomcat/trunk/java/org/apache/catalina/ha/tcp/ReplicationValve.java Fri Feb 23 14:43:40 2018 @@ -93,13 +93,21 @@ public class ReplicationValve */ protected boolean doProcessingStats = false; - protected long totalRequestTime = 0; - protected long totalSendTime = 0; - protected long nrOfRequests = 0; - protected long lastSendTime = 0; - protected long nrOfFilterRequests = 0; - protected long nrOfSendRequests = 0; - protected long nrOfCrossContextSendRequests = 0; + /* + * Note: The statistics are volatile to ensure the concurrent updates do not + * corrupt them but it is still possible that: + * - some updates may be lost; + * - the individual statistics may not be consistent which each other. + * This is a deliberate design choice to reduce the requirement for + * synchronization. + */ + protected volatile long totalRequestTime = 0; + protected volatile long totalSendTime = 0; + protected volatile long nrOfRequests = 0; + protected volatile long lastSendTime = 0; + protected volatile long nrOfFilterRequests = 0; + protected volatile long nrOfSendRequests = 0; + protected volatile long nrOfCrossContextSendRequests = 0; /** * must primary change indicator set @@ -354,11 +362,11 @@ public class ReplicationValve * reset the active statistics */ public void resetStatistics() { - totalRequestTime = 0 ; - totalSendTime = 0 ; - lastSendTime = 0 ; - nrOfFilterRequests = 0 ; - nrOfRequests = 0 ; + totalRequestTime = 0; + totalSendTime = 0; + lastSendTime = 0; + nrOfFilterRequests = 0; + nrOfRequests = 0; nrOfSendRequests = 0; nrOfCrossContextSendRequests = 0; } @@ -563,12 +571,11 @@ public class ReplicationValve protected void updateStats(long requestTime, long clusterTime) { // TODO: Async requests may trigger multiple replication requests. How, // if at all, should the stats handle this? - synchronized(this) { - lastSendTime=System.currentTimeMillis(); - totalSendTime+=lastSendTime - clusterTime; - totalRequestTime+=lastSendTime - requestTime; - nrOfRequests++; - } + long currentTime = System.currentTimeMillis(); + lastSendTime = currentTime; + totalSendTime += currentTime - clusterTime; + totalRequestTime += currentTime - requestTime; + nrOfRequests++; if(log.isInfoEnabled()) { if ( (nrOfRequests % 100) == 0 ) { log.info(sm.getString("ReplicationValve.stats", Modified: tomcat/trunk/res/findbugs/filter-false-positives.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/res/findbugs/filter-false-positives.xml?rev=1825130&r1=1825129&r2=1825130&view=diff ============================================================================== --- tomcat/trunk/res/findbugs/filter-false-positives.xml (original) +++ tomcat/trunk/res/findbugs/filter-false-positives.xml Fri Feb 23 14:43:40 2018 @@ -259,16 +259,27 @@ <Match> <!-- False positive caused by additional method syncs --> <Class name="org.apache.catalina.ha.session.DeltaManager" /> - <Field name="receiverQueue" /> + <Field name="receiverQueue" /> <Pattern code="IS2_INCONSISTENT_SYNC" /> </Match> <Match> <!-- False positive caused by method syncs --> <Class name="org.apache.catalina.ha.session.JvmRouteBinderValve" /> - <Field name="cluster" /> + <Field name="cluster" /> <Pattern code="IS2_INCONSISTENT_SYNC" /> </Match> <Match> + <!-- Design choice to reduce need for syncs --> + <Class name="org.apache.catalina.ha.tcp.ReplicationValve" /> + <Or> + <Field name="nrOfCrossContextSendRequests" /> + <Field name="nrOfFilterRequests" /> + <Field name="nrOfRequests" /> + <Field name="nrOfSendRequests" /> + </Or> + <Pattern code="VO_VOLATILE_INCREMENT" /> + </Match> + <Match> <!-- Field is only modified during Servlet load --> <Class name="org.apache.catalina.manager.host.HostManagerServlet" /> <Bug code="MSF" /> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org