[
https://issues.apache.org/jira/browse/GEODE-6889?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Darrel Schneider resolved GEODE-6889.
-------------------------------------
Resolution: Fixed
Fix Version/s: 1.10.0
> replyWaitMaxTime stat calculation code needs improvement
> --------------------------------------------------------
>
> Key: GEODE-6889
> URL: https://issues.apache.org/jira/browse/GEODE-6889
> Project: Geode
> Issue Type: Improvement
> Components: core
> Reporter: Darrel Schneider
> Assignee: Murtuza Boxwala
> Priority: Major
> Labels: performance
> Fix For: 1.10.0
>
> Time Spent: 4h 40m
> Remaining Estimate: 0h
>
> The way the replyWaitMaxTime is calculated has some problems:
> 1. the elapsed time is calculated using System.currentTimeMillis but should
> instead use getStatTime (it actually uses getStatTime for the begin timestamp
> but not for the end timestamp).
> 2. a local atomic should be used to calculate it instead of reading the value
> from the Statistics instance. Reading is expensive since it uses
> synchronization.
> The following is a prototype fix for #2 I used when performance testing:
> {noformat}
> diff --git
> a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionStats.java
>
> b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionStats.java
> index 91c47e2495..d591e35570 100644
> ---
> a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionStats.java
> +++
> b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionStats.java
> @@ -14,6 +14,8 @@
> */
> package org.apache.geode.distributed.internal;
>
> +import java.util.concurrent.atomic.AtomicLong;
> +
> import org.apache.logging.log4j.Logger;
>
> import org.apache.geode.StatisticDescriptor;
> @@ -1602,6 +1604,8 @@ public class DistributionStats implements DMStats {
> return getStatTime();
> }
>
> + private final AtomicLong replyWaitMaxTime = new AtomicLong();
> +
> @Override
> public void endReplyWait(long startNanos, long initTime) {
> if (enableClockStats) {
> @@ -1610,8 +1614,17 @@ public class DistributionStats implements DMStats {
> }
> if (initTime != 0) {
> long mswait = System.currentTimeMillis() - initTime;
> - if (mswait > getReplyWaitMaxTime()) {
> - stats.setLong(replyWaitMaxTimeId, mswait);
> + boolean done = false;
> + while (!done) {
> + long currentReplyWaitMaxTime = replyWaitMaxTime.get();
> + if (mswait > currentReplyWaitMaxTime) {
> + done = replyWaitMaxTime.compareAndSet(currentReplyWaitMaxTime,
> mswait);
> + if (done) {
> + stats.setLong(replyWaitMaxTimeId, mswait);
> + }
> + } else {
> + done = true;
> + }
> }
> }
> stats.incInt(replyWaitsInProgressId, -1);{noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
