Merge branch '2.1.x'

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/WeightCalculatorWebFilter.java

@@ -170,47 +170,57 @@ public class WeightCalculatorWebFilter
 
 	/* for testing */ void addWeightConfig(WeightConfig weightConfig) {
 		String group = weightConfig.getGroup();
-		GroupWeightConfig c = groupWeights.get(group);
-		if (c == null) {
-			c = new GroupWeightConfig(group);
-			groupWeights.put(group, c);
+		GroupWeightConfig config;
+		// only create new GroupWeightConfig rather than modify
+		// and put at end of calculations. This avoids concurency problems
+		// later during filter execution.
+		if (groupWeights.containsKey(group)) {
+			config = new GroupWeightConfig(groupWeights.get(group));
+		}
+		else {
+			config = new GroupWeightConfig(group);
 		}
-		GroupWeightConfig config = c;
-		synchronized (config) {
-			config.weights.put(weightConfig.getRouteId(), weightConfig.getWeight());
 
-			// recalculate
+		config.weights.put(weightConfig.getRouteId(), weightConfig.getWeight());
 
-			// normalize weights
-			int weightsSum = config.weights.values().stream().mapToInt(Integer::intValue)
-					.sum();
+		// recalculate
 
-			final AtomicInteger index = new AtomicInteger(0);
-			config.weights.forEach((routeId, weight) -> {
-				Double nomalizedWeight = weight / (double) weightsSum;
-				config.normalizedWeights.put(routeId, nomalizedWeight);
+		// normalize weights
+		int weightsSum = 0;
 
-				// recalculate rangeIndexes
-				config.rangeIndexes.put(index.getAndIncrement(), routeId);
-			});
+		for (Integer weight : config.weights.values()) {
+			weightsSum += weight;
+		}
 
-			// TODO: calculate ranges
-			config.ranges.clear();
+		final AtomicInteger index = new AtomicInteger(0);
+		for (Map.Entry<String, Integer> entry : config.weights.entrySet()) {
+			String routeId = entry.getKey();
+			Integer weight = entry.getValue();
+			Double nomalizedWeight = weight / (double) weightsSum;
+			config.normalizedWeights.put(routeId, nomalizedWeight);
 
-			config.ranges.add(0.0);
+			// recalculate rangeIndexes
+			config.rangeIndexes.put(index.getAndIncrement(), routeId);
+		}
 
-			List<Double> values = new ArrayList<>(config.normalizedWeights.values());
-			for (int i = 0; i < values.size(); i++) {
-				Double currentWeight = values.get(i);
-				Double previousRange = config.ranges.get(i);
-				Double range = previousRange + currentWeight;
-				config.ranges.add(range);
-			}
+		// TODO: calculate ranges
+		config.ranges.clear();
 
-			if (log.isTraceEnabled()) {
-				log.trace("Recalculated group weight config " + config);
-			}
+		config.ranges.add(0.0);
+
+		List<Double> values = new ArrayList<>(config.normalizedWeights.values());
+		for (int i = 0; i < values.size(); i++) {
+			Double currentWeight = values.get(i);
+			Double previousRange = config.ranges.get(i);
+			Double range = previousRange + currentWeight;
+			config.ranges.add(range);
 		}
+
+		if (log.isTraceEnabled()) {
+			log.trace("Recalculated group weight config " + config);
+		}
+		// only update after all calculations
+		groupWeights.put(group, config);
 	}
 
 	/* for testing */ Map<String, GroupWeightConfig> getGroupWeights() {
@@ -233,20 +243,18 @@ public class WeightCalculatorWebFilter
 
 			double r = this.random.nextDouble();
 
-			synchronized (config) {
-				List<Double> ranges = config.ranges;
+			List<Double> ranges = config.ranges;
 
-				if (log.isTraceEnabled()) {
-					log.trace("Weight for group: " + group + ", ranges: " + ranges
-							+ ", r: " + r);
-				}
+			if (log.isTraceEnabled()) {
+				log.trace("Weight for group: " + group + ", ranges: " + ranges + ", r: "
+						+ r);
+			}
 
-				for (int i = 0; i < ranges.size() - 1; i++) {
-					if (r >= ranges.get(i) && r < ranges.get(i + 1)) {
-						String routeId = config.rangeIndexes.get(i);
-						weights.put(group, routeId);
-						break;
-					}
+			for (int i = 0; i < ranges.size() - 1; i++) {
+				if (r >= ranges.get(i) && r < ranges.get(i + 1)) {
+					String routeId = config.rangeIndexes.get(i);
+					weights.put(group, routeId);
+					break;
 				}
 			}
 		}
@@ -274,6 +282,13 @@ public class WeightCalculatorWebFilter
 			this.group = group;
 		}
 
+		GroupWeightConfig(GroupWeightConfig other) {
+			this.group = other.group;
+			this.weights = new LinkedHashMap<>(other.weights);
+			this.normalizedWeights = new LinkedHashMap<>(other.normalizedWeights);
+			this.rangeIndexes = new LinkedHashMap<>(other.rangeIndexes);
+		}
+
 		@Override
 		public String toString() {
 			return new ToStringCreator(this).append("group", group)

spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/WeightCalculatorWebFilterConcurrentTests.java

@@ -23,7 +23,6 @@ import java.util.concurrent.TimeUnit;
 
 import io.netty.util.internal.ThreadLocalRandom;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mockito;
@@ -58,7 +57,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen
 @DirtiesContext
 public class WeightCalculatorWebFilterConcurrentTests {
 
-	@Value("${test.concurrent.execution.timeInSeconds}")
+	@Value("${test.concurrent.execution.timeInSeconds:5}")
 	private int maxTestTimeSeconds;
 
 	@Autowired
@@ -77,7 +76,6 @@ public class WeightCalculatorWebFilterConcurrentTests {
 		startTime = System.currentTimeMillis();
 	}
 
-	@Ignore
 	@Test
 	public void WeightCalculatorWebFilter_threadSafeTest() {
 		generateEvents();