Skip to content
This repository was archived by the owner on May 28, 2018. It is now read-only.

Commit 591a613

Browse files
stepanvstepanv
committed
J-585: Fixed a problem with disappearing values in Jersey Monitoring internals - there was a race condition in the trimming algorithm of the aggregating trimmer
Cherry-picked from master:8d959e9d673e07e47bde1ab95fe152637c0bd6fb Change-Id: Icb51bca8272ae785909375d5375053a1050019e9
1 parent b69d531 commit 591a613

File tree

6 files changed

+390
-25
lines changed

6 files changed

+390
-25
lines changed

‎core-server/src/main/java/org/glassfish/jersey/server/internal/monitoring/AggregatedValueObject.java‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
package org.glassfish.jersey.server.internal.monitoring;
4141

4242
import java.util.Collection;
43+
import java.util.LinkedList;
4344

4445
/**
4546
* Aggregated value object stores aggregated measurements for provided set of data. The purpose of aggregation is to avoid high
@@ -54,14 +55,21 @@ class AggregatedValueObject {
5455
private final double mean;
5556
private final long count;
5657

58+
private AggregatedValueObject(final long max, final long min, final double mean, final long count) {
59+
this.max = max;
60+
this.min = min;
61+
this.mean = mean;
62+
this.count = count;
63+
}
64+
5765
/**
5866
* Creates aggregated value object for monitoring statistics based on the provided values. During the construction, the values
5967
* collection must not be modified.
6068
*
61-
* @param values The collection to create the aggregated statistics from
69+
* @param values The collection to create the aggregated statistics from.
70+
* @return Aggregated value object for provided arguments.
6271
*/
63-
public AggregatedValueObject(final Collection<Long> values) {
64-
72+
public static AggregatedValueObject createFromValues(Collection<Long> values) {
6573
if (values.isEmpty()) {
6674
// aggregated objects must be created for at least one value, additionally, prevent from division by zero in the mean
6775
throw new IllegalArgumentException("The values collection must not be empty");
@@ -76,10 +84,22 @@ public AggregatedValueObject(final Collection<Long> values) {
7684
sum += value;
7785
}
7886

79-
this.min = min;
80-
this.max = max;
81-
this.count = values.size();
82-
this.mean = (double) sum / count;
87+
return new AggregatedValueObject(max, min, (double) sum / values.size(), values.size());
88+
}
89+
90+
/**
91+
* Creates aggregated value object for monitoring statistics based on the provided collection of values. During the
92+
* construction, the values collection must not be modified.
93+
*
94+
* @param values The collection to create the aggregated statistics from.
95+
* @return Aggregated value object for provided arguments.
96+
*/
97+
public static AggregatedValueObject createFromMultiValues(Collection<? extends Collection<Long>> values) {
98+
final Collection<Long> mergedCollection = new LinkedList<>();
99+
for (Collection<Long> collection : values) {
100+
mergedCollection.addAll(collection);
101+
}
102+
return createFromValues(mergedCollection);
83103
}
84104

85105
/**

‎core-server/src/main/java/org/glassfish/jersey/server/internal/monitoring/AggregatingTrimmer.java‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,20 @@
3939
*/
4040
package org.glassfish.jersey.server.internal.monitoring;
4141

42+
import java.util.Collection;
4243
import java.util.List;
4344
import java.util.Map;
4445
import java.util.NavigableMap;
45-
import java.util.TreeMap;
46+
import java.util.SortedMap;
4647
import java.util.concurrent.ConcurrentNavigableMap;
4748
import java.util.concurrent.CopyOnWriteArrayList;
4849
import java.util.concurrent.TimeUnit;
4950
import java.util.concurrent.atomic.AtomicBoolean;
5051

5152
import static org.glassfish.jersey.server.internal.monitoring.ReservoirConstants.COLLISION_BUFFER_POWER;
5253

54+
import jersey.repackaged.com.google.common.collect.TreeMultimap;
55+
5356
/**
5457
* An aggregating trimmer for sliding window measurements. This trimmer updates registered time reservoirs with the aggregated
5558
* measurements for the values it trimmed.
@@ -71,12 +74,12 @@ class AggregatingTrimmer implements SlidingWindowTrimmer<Long> {
7174
private final AtomicBoolean locked = new AtomicBoolean(false);
7275

7376
/**
74-
* Creates the trimmer that updates the registered time reservoirs with the aggregated measurements
75-
* for the values it trimmed.
77+
* Creates the trimmer that updates the registered time reservoirs with the aggregated measurements for the values it
78+
* trimmed.
7679
*
77-
* @param startTime The start time that determines the offset for the chunks.
78-
* @param startUnitTime The time unit of the start time.
79-
* @param chunkTimeSize The size of one "time chunk".
80+
* @param startTime The start time that determines the offset for the chunks.
81+
* @param startUnitTime The time unit of the start time.
82+
* @param chunkTimeSize The size of one "time chunk".
8083
* @param chunkTimeSizeUnit The time unit of the time chunk.
8184
*/
8285
public AggregatingTrimmer(final long startTime,
@@ -93,28 +96,30 @@ public void trim(final ConcurrentNavigableMap<Long, Long> map, final long key) {
9396
if (!locked.compareAndSet(false, true)) {
9497
return;
9598
}
96-
final NavigableMap<Long, Long> trimMap;
99+
final TreeMultimap<Long, Long> trimMultiMap = TreeMultimap.create();
100+
final NavigableMap<Long, Collection<Long>> trimMap = trimMultiMap.asMap();
97101

98102
try {
99-
trimMap = new TreeMap<>();
100103
final ConcurrentNavigableMap<Long, Long> headMap = map.headMap(key);
101104
while (!headMap.isEmpty()) {
105+
// headMap itself is being accessed with updates from other threads
102106
final Map.Entry<Long, Long> entry = headMap.pollFirstEntry();
103-
trimMap.put(entry.getKey(), entry.getValue());
107+
trimMultiMap.put(entry.getKey(), entry.getValue());
104108
}
105109
// now the headMap is trimmed...
106110
} finally {
107111
locked.set(false);
108112
}
109113

110-
for (Map.Entry<Long, Long> firstEntry = trimMap.firstEntry(); firstEntry != null; firstEntry = trimMap.firstEntry()) {
111-
long chunkLowerBound = lowerBound(firstEntry);
114+
for (Map.Entry<Long, Collection<Long>> firstEntry = trimMap.firstEntry(); firstEntry != null;
115+
firstEntry = trimMap.firstEntry()) {
116+
long chunkLowerBound = lowerBound(firstEntry.getKey());
112117
long chunkUpperBound = upperBound(chunkLowerBound, key);
113118

114119
// now, we need to process and then remove entries at interval [chunkLowerBound, chunkUpperBound)
115-
Map<Long, Long> chunkMap = trimMap.headMap(chunkUpperBound);
120+
SortedMap<Long, Collection<Long>> chunkMap = trimMap.headMap(chunkUpperBound);
116121

117-
final AggregatedValueObject aggregatedValueObject = new AggregatedValueObject(chunkMap.values());
122+
final AggregatedValueObject aggregatedValueObject = AggregatedValueObject.createFromMultiValues(chunkMap.values());
118123

119124
// update all listening aggregated reservoirs
120125
for (TimeReservoir<AggregatedValueObject> aggregatedReservoir : aggregatedReservoirListeners) {
@@ -132,8 +137,8 @@ private long upperBound(final long chunkLowerBound, final long key) {
132137
return chunkUpperBoundCandidate < key ? chunkUpperBoundCandidate : key;
133138
}
134139

135-
private long lowerBound(final Map.Entry<Long, Long> firstEntry) {
136-
return lowerBound(firstEntry.getKey(), TimeUnit.NANOSECONDS.convert(startTime, startUnitTime), chunkSize,
140+
private long lowerBound(final Long key) {
141+
return lowerBound(key, TimeUnit.NANOSECONDS.convert(startTime, startUnitTime), chunkSize,
137142
COLLISION_BUFFER_POWER);
138143
}
139144

‎core-server/src/test/java/org/glassfish/jersey/server/internal/monitoring/AbstractNanosReservoirTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ protected void checkInNanos(final TimeReservoir reservoir,
7878
*/
7979
protected void checkInNanos(final TimeReservoir reservoir,
8080
final long snapshotTime,
81-
final int expectedSize,
82-
final int expectedMin,
83-
final int expectedMax,
81+
final long expectedSize,
82+
final long expectedMin,
83+
final long expectedMax,
8484
final double expectedMean, final long expectedInterval) {
8585
final UniformTimeSnapshot snapshot = reservoir.getSnapshot(snapshotTime, TimeUnit.NANOSECONDS);
8686

‎tests/pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
<module>integration</module>
6666
<module>mem-leaks</module>
6767
<module>osgi</module>
68+
<module>stress</module>
6869
<module>performance</module>
6970
</modules>
7071

‎tests/stress/pom.xml‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
4+
DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
5+
6+
Copyright (c) 2015 Oracle and/or its affiliates. All rights reserved.
7+
8+
The contents of this file are subject to the terms of either the GNU
9+
General Public License Version 2 only ("GPL") or the Common Development
10+
and Distribution License("CDDL") (collectively, the "License"). You
11+
may not use this file except in compliance with the License. You can
12+
obtain a copy of the License at
13+
http://glassfish.java.net/public/CDDL+GPL_1_1.html
14+
or packager/legal/LICENSE.txt. See the License for the specific
15+
language governing permissions and limitations under the License.
16+
17+
When distributing the software, include this License Header Notice in each
18+
file and include the License file at packager/legal/LICENSE.txt.
19+
20+
GPL Classpath Exception:
21+
Oracle designates this particular file as subject to the "Classpath"
22+
exception as provided by Oracle in the GPL Version 2 section of the License
23+
file that accompanied this code.
24+
25+
Modifications:
26+
If applicable, add the following below the License Header, with the fields
27+
enclosed by brackets [] replaced by your own identifying information:
28+
"Portions Copyright [year] [name of copyright owner]"
29+
30+
Contributor(s):
31+
If you wish your version of this file to be governed by only the CDDL or
32+
only the GPL Version 2, indicate your decision by adding "[Contributor]
33+
elects to include this software in this distribution under the [CDDL or GPL
34+
Version 2] license." If you don't indicate a single choice of license, a
35+
recipient has the option to distribute your version of this file under
36+
either the CDDL, the GPL Version 2 or to extend the choice of license to
37+
its licensees as provided above. However, if you add GPL Version 2 code
38+
and therefore, elected the GPL Version 2 license, then the option applies
39+
only if the new code is made subject to such option by the copyright
40+
holder.
41+
42+
-->
43+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
44+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
45+
<modelVersion>4.0.0</modelVersion>
46+
47+
<parent>
48+
<groupId>org.glassfish.jersey.tests</groupId>
49+
<artifactId>project</artifactId>
50+
<version>2.21.1-SNAPSHOT</version>
51+
</parent>
52+
53+
<artifactId>stress</artifactId>
54+
<packaging>jar</packaging>
55+
<name>jersey-tests-stress</name>
56+
57+
<description>
58+
59+
Jersey stress tests
60+
61+
This module may contain tests which execution is limited by time (or a fault). This may lead to a considerable longer
62+
build of Jersey.
63+
64+
</description>
65+
66+
<build>
67+
<plugins>
68+
<plugin>
69+
<groupId>org.apache.maven.plugins</groupId>
70+
<artifactId>maven-compiler-plugin</artifactId>
71+
</plugin>
72+
<plugin>
73+
<groupId>org.apache.maven.plugins</groupId>
74+
<artifactId>maven-surefire-plugin</artifactId>
75+
<configuration>
76+
<argLine>-Xmx2048m</argLine>
77+
<forkMode>always</forkMode>
78+
<enableAssertions>false</enableAssertions>
79+
</configuration>
80+
</plugin>
81+
</plugins>
82+
</build>
83+
84+
<dependencies>
85+
<dependency>
86+
<groupId>org.glassfish.jersey.test-framework.providers</groupId>
87+
<artifactId>jersey-test-framework-provider-bundle</artifactId>
88+
<type>pom</type>
89+
<scope>test</scope>
90+
</dependency>
91+
92+
<dependency>
93+
<groupId>com.google.guava</groupId>
94+
<artifactId>guava</artifactId>
95+
<version>${guava.version}</version>
96+
<scope>test</scope>
97+
</dependency>
98+
99+
<dependency>
100+
<groupId>org.glassfish.jersey.test-framework</groupId>
101+
<artifactId>jersey-test-framework-util</artifactId>
102+
<scope>test</scope>
103+
</dependency>
104+
105+
<dependency>
106+
<groupId>org.hamcrest</groupId>
107+
<artifactId>hamcrest-library</artifactId>
108+
<scope>test</scope>
109+
</dependency>
110+
</dependencies>
111+
112+
</project>

0 commit comments

Comments
 (0)