Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add thread safety to stats #719

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# The agent version.
agentVersion=7.6.0
agentVersion=7.6.0-13353-2

newrelicDebug=false
org.gradle.jvmargs=-Xmx2048m
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.io.Writer;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;

import org.json.simple.JSONArray;

Expand All @@ -24,7 +25,7 @@ public abstract class AbstractStats implements CountStats {
Number zero = 0;
ZERO_ARRAY_LIST = Arrays.asList(zero, zero, zero, zero, zero, zero);
}
protected int count;
private final AtomicInteger count = new AtomicInteger(0);

/**
* Used when we want to send up a metric with a zero call count. Not available in the public api.
Expand Down Expand Up @@ -61,33 +62,33 @@ public AbstractStats() {

public AbstractStats(int count) {
super();
this.count = count;
this.count.set(count);
}

@Override
public void incrementCallCount(int value) {
count += value;
this.count.addAndGet(value);
}

@Override
public void incrementCallCount() {
this.count++;
this.count.incrementAndGet();
}

@Override
public int getCallCount() {
return count;
return count.get();
}

@Override
public void setCallCount(int count) {
this.count = count;
this.count.set(count);
}

@Override
public final void writeJSONString(Writer writer) throws IOException, InvalidStatsException {
List<Number> list;
if (count < 0) {
if (count.get() < 0) {
list = ZERO_ARRAY_LIST;
} else {
list = Arrays.asList(count, getTotal(), getTotalExclusiveTime(), getMinCallTime(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class ResponseTimeStatsImpl extends AbstractStats implements ResponseTime
private static final long NANOSECONDS_PER_SECOND_SQUARED = TimeConversion.NANOSECONDS_PER_SECOND
* TimeConversion.NANOSECONDS_PER_SECOND;

private final Object lock = new Object();

private long total;
private long totalExclusive;
private long minValue;
Expand All @@ -35,14 +37,7 @@ protected ResponseTimeStatsImpl() {

@Override
public Object clone() throws CloneNotSupportedException {
ResponseTimeStatsImpl newStats = new ResponseTimeStatsImpl();
newStats.count = count;
newStats.total = total;
newStats.totalExclusive = totalExclusive;
newStats.minValue = minValue;
newStats.maxValue = maxValue;
newStats.sumOfSquares = sumOfSquares;
return newStats;
return copy();
}

@Override
Expand All @@ -67,94 +62,129 @@ public void recordResponseTimeInNanos(long responseTime) {
public void recordResponseTimeInNanos(long responseTime, long exclusiveTime) {
double responseTimeAsDouble = responseTime;
responseTimeAsDouble *= responseTimeAsDouble;
sumOfSquares += responseTimeAsDouble;
if (count > 0) {
minValue = Math.min(responseTime, minValue);
} else {
minValue = responseTime;
}
count++;
total += responseTime;
maxValue = Math.max(responseTime, maxValue);
totalExclusive += exclusiveTime;
if (NewRelic.getAgent().getConfig().getValue(AgentConfigImpl.METRIC_DEBUG, AgentConfigImpl.DEFAULT_METRIC_DEBUG)) {
if (count < 0 || total < 0 || totalExclusive < 0 || sumOfSquares < 0) {
NewRelic.incrementCounter("Supportability/ResponseTimeStatsImpl/NegativeValue");
Agent.LOG.log(Level.INFO, "Invalid count {0}, total {1}, totalExclusive {2}, or sum of squares {3}",
count, total, totalExclusive, sumOfSquares);
synchronized (lock) {
sumOfSquares += responseTimeAsDouble;
if (getCallCount() > 0) {
minValue = Math.min(responseTime, minValue);
} else {
minValue = responseTime;
}
incrementCallCount();
total += responseTime;
maxValue = Math.max(responseTime, maxValue);
totalExclusive += exclusiveTime;
if (NewRelic.getAgent().getConfig().getValue(AgentConfigImpl.METRIC_DEBUG, AgentConfigImpl.DEFAULT_METRIC_DEBUG)) {
if (getCallCount() < 0 || total < 0 || totalExclusive < 0 || sumOfSquares < 0) {
NewRelic.incrementCounter("Supportability/ResponseTimeStatsImpl/NegativeValue");
Agent.LOG.log(Level.INFO, "Invalid count {0}, total {1}, totalExclusive {2}, or sum of squares {3}",
getCallCount(), total, totalExclusive, sumOfSquares);
}
}
}

}

@Override
public boolean hasData() {
return count > 0 || total > 0 || totalExclusive > 0;
boolean hasData;
synchronized (lock) {
hasData = getCallCount() > 0 || total > 0 || totalExclusive > 0;
}
return hasData;
}

@Override
public void reset() {
count = 0;
total = totalExclusive = minValue = maxValue = 0;
sumOfSquares = 0;
synchronized (lock) {
setCallCount(0);
total = totalExclusive = minValue = maxValue = 0;
sumOfSquares = 0;
}
}

@Override
public float getTotal() {
return (float) total / TimeConversion.NANOSECONDS_PER_SECOND;
synchronized (lock) {
return (float) total / TimeConversion.NANOSECONDS_PER_SECOND;
}
}

@Override
public float getTotalExclusiveTime() {
return (float) totalExclusive / TimeConversion.NANOSECONDS_PER_SECOND;
synchronized (lock) {
return (float) totalExclusive / TimeConversion.NANOSECONDS_PER_SECOND;
}
}


@Override
public float getMaxCallTime() {
return (float) maxValue / TimeConversion.NANOSECONDS_PER_SECOND;
synchronized (lock) {
return (float) maxValue / TimeConversion.NANOSECONDS_PER_SECOND;
}
}

@Override
public float getMinCallTime() {
return (float) minValue / TimeConversion.NANOSECONDS_PER_SECOND;
synchronized (lock) {
return (float) minValue / TimeConversion.NANOSECONDS_PER_SECOND;
}
}

@Override
public double getSumOfSquares() {
return sumOfSquares / NANOSECONDS_PER_SECOND_SQUARED;
synchronized (lock) {
return sumOfSquares / NANOSECONDS_PER_SECOND_SQUARED;
}
}

private ResponseTimeStatsImpl copy() {
ResponseTimeStatsImpl newStats = new ResponseTimeStatsImpl();
synchronized (lock) {
newStats.setCallCount(this.getCallCount());
newStats.total = total;
newStats.totalExclusive = totalExclusive;
newStats.minValue = minValue;
newStats.maxValue = maxValue;
newStats.sumOfSquares = sumOfSquares;
}
return newStats;
}

@Override
public final void merge(StatsBase statsObj) {
if (statsObj instanceof ResponseTimeStatsImpl) {
ResponseTimeStatsImpl stats = (ResponseTimeStatsImpl) statsObj;
if (stats.count > 0) {
if (count > 0) {
minValue = Math.min(minValue, stats.minValue);
} else {
minValue = stats.minValue;
ResponseTimeStatsImpl stats = ((ResponseTimeStatsImpl) statsObj).copy();
synchronized (lock) {
if (stats.getCallCount() > 0) {
if (getCallCount() > 0) {
minValue = Math.min(minValue, stats.minValue);
} else {
minValue = stats.minValue;
}
}
}
count += stats.count;
total += stats.total;
totalExclusive += stats.totalExclusive;
incrementCallCount(stats.getCallCount());
total += stats.total;
totalExclusive += stats.totalExclusive;

maxValue = Math.max(maxValue, stats.maxValue);
sumOfSquares += stats.sumOfSquares;
maxValue = Math.max(maxValue, stats.maxValue);
sumOfSquares += stats.sumOfSquares;
}
}
}

@Override
public void recordResponseTime(int count, long totalTime, long minTime, long maxTime, TimeUnit unit) {
long totalTimeInNanos = TimeUnit.NANOSECONDS.convert(totalTime, unit);
this.count = count;
this.total = totalTimeInNanos;
this.totalExclusive = totalTimeInNanos;
this.minValue = TimeUnit.NANOSECONDS.convert(minTime, unit);
this.maxValue = TimeUnit.NANOSECONDS.convert(maxTime, unit);
double totalTimeInNanosAsDouble = totalTimeInNanos;
totalTimeInNanosAsDouble *= totalTimeInNanosAsDouble;
sumOfSquares += totalTimeInNanosAsDouble;
synchronized (lock) {
long totalTimeInNanos = TimeUnit.NANOSECONDS.convert(totalTime, unit);
this.setCallCount(count);
this.total = totalTimeInNanos;
this.totalExclusive = totalTimeInNanos;
this.minValue = TimeUnit.NANOSECONDS.convert(minTime, unit);
this.maxValue = TimeUnit.NANOSECONDS.convert(maxTime, unit);
double totalTimeInNanosAsDouble = totalTimeInNanos;
totalTimeInNanosAsDouble *= totalTimeInNanosAsDouble;
sumOfSquares += totalTimeInNanosAsDouble;
}
}

@Override
Expand Down
Loading