From bbf50844329b330ca4eb8198de6c43ea1b2a0451 Mon Sep 17 00:00:00 2001 From: yipk Date: Mon, 5 Dec 2016 13:46:44 +0800 Subject: [PATCH 1/2] AtomicInteger.incrementAndGet() is likely overflow AtomicInteger(Integer.MAX_VALUE).incrementAndGet() returns -2147483648 --- .../loadbalancer/AbstractServerPredicate.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/AbstractServerPredicate.java b/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/AbstractServerPredicate.java index 62186a62..408f6431 100644 --- a/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/AbstractServerPredicate.java +++ b/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/AbstractServerPredicate.java @@ -137,6 +137,22 @@ public List getEligibleServers(List servers, Object loadBalancer return results; } } + + /** + * Referenced from RoundRobinRule + * Inspired by the implementation of {@link AtomicInteger#incrementAndGet()}. + * + * @param modulo The modulo to bound the value of the counter. + * @return The next value. + */ + private int incrementAndGetModulo(int modulo) { + for (;;) { + int current = nextIndex.get(); + int next = (current + 1) % modulo; + if (nextIndex.compareAndSet(current, next)) + return next; + } + } /** * Choose a random server after the predicate filters a list of servers. Load balancer key @@ -160,7 +176,7 @@ public Optional chooseRoundRobinAfterFiltering(List servers) { if (eligible.size() == 0) { return Optional.absent(); } - return Optional.of(eligible.get(nextIndex.getAndIncrement() % eligible.size())); + return Optional.of(eligible.get(incrementAndGetModulo(eligible.size()))); } /** @@ -184,7 +200,7 @@ public Optional chooseRoundRobinAfterFiltering(List servers, Obj if (eligible.size() == 0) { return Optional.absent(); } - return Optional.of(eligible.get(nextIndex.getAndIncrement() % eligible.size())); + return Optional.of(eligible.get(incrementAndGetModulo(eligible.size()))); } /** From 53a2c7dc6dbd94959510f4a2e697657b276ab0f7 Mon Sep 17 00:00:00 2001 From: yipk Date: Tue, 6 Dec 2016 18:37:40 +0800 Subject: [PATCH 2/2] change to return current to make unit test pass --- .../java/com/netflix/loadbalancer/AbstractServerPredicate.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/AbstractServerPredicate.java b/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/AbstractServerPredicate.java index 408f6431..c4bc11ca 100644 --- a/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/AbstractServerPredicate.java +++ b/ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/AbstractServerPredicate.java @@ -150,7 +150,7 @@ private int incrementAndGetModulo(int modulo) { int current = nextIndex.get(); int next = (current + 1) % modulo; if (nextIndex.compareAndSet(current, next)) - return next; + return current; } }