Bug Report
For English only, other languages will not accept.
Which version of ShardingSphere did you use?
master (latest)
Which project did you use? ShardingSphere-JDBC or ShardingSphere-Proxy?
Both — RoundRobinLoadBalanceAlgorithm lives in infra and is the default ROUND_ROBIN load balancer used by readwrite-splitting.
Expected behavior
RoundRobinLoadBalanceAlgorithm.getTargetName() keeps returning a valid target name for the lifetime of the process.
Actual behavior
Once the internal counter passes Integer.MAX_VALUE, every call throws ArrayIndexOutOfBoundsException for any replica group whose size is not a power of two.
java.lang.ArrayIndexOutOfBoundsException: Index -2 out of bounds for length 3
at RoundRobinLoadBalanceAlgorithm.getTargetName(RoundRobinLoadBalanceAlgorithm.java:36)
Reason analyze (If you can)
RoundRobinLoadBalanceAlgorithm.java:36:
return availableTargetNames.get(Math.abs(count.getAndIncrement()) % availableTargetNames.size());
Math.abs(Integer.MIN_VALUE) is Integer.MIN_VALUE — abs cannot represent 2^31 in an int, so it returns the negative value unchanged. The remainder is then negative and List.get throws:
| group size |
Math.abs(Integer.MIN_VALUE) % size |
|
| 2 |
0 |
ok |
| 3 |
-2 |
throws |
| 4 |
0 |
ok |
| 5 |
-3 |
throws |
| 8 |
0 |
ok |
Power-of-two group sizes happen to yield 0 and survive, which is why a 2-replica setup never shows it. getTargetName is annotated @HighFrequencyInvocation and the counter is never reset, so a long-lived proxy handling read traffic reaches the wrap on its own.
There is prior art for this exact failure: #1265 reported the same ArrayIndexOutOfBoundsException in 2018 and was closed as completed. The fix at the time was a count.compareAndSet(readDataSourceNames.size(), 0) line that kept the counter bounded. That line was later removed in #17422 while the counter was being changed from a static map to an instance field, and the Math.abs(...) expression was kept. (For the record, that old line was itself racy under concurrency — several threads could step past size before any of them observed it — so the point is not that a good guard was lost, but that the current code has no bound on the counter at all.)
Steps to reproduce the behavior
Set the counter to Integer.MAX_VALUE and call the algorithm twice against a 3-element list:
LoadBalanceAlgorithm algorithm = TypedSPILoader.getService(LoadBalanceAlgorithm.class, "ROUND_ROBIN", new Properties());
Plugins.getMemberAccessor().set(
RoundRobinLoadBalanceAlgorithm.class.getDeclaredField("count"), algorithm, new AtomicInteger(Integer.MAX_VALUE));
List<String> availableTargetNames = Arrays.asList("ds_1", "ds_2", "ds_3");
algorithm.getTargetName("ds", availableTargetNames); // ok, index 1
algorithm.getTargetName("ds", availableTargetNames); // ArrayIndexOutOfBoundsException: Index -2 out of bounds for length 3
Example codes for reproduce this issue (such as a github link)
The snippet above runs as-is inside RoundRobinLoadBalanceAlgorithmTest. I have a fix and a regression test ready and will open a PR referencing this issue.
Math.floorMod(count.getAndIncrement(), availableTargetNames.size()) is a one-line replacement that is identical to the current expression for every non-negative counter value — I verified that exhaustively for all counters in [0, 5_000_000] across group sizes 1..16 — and returns a valid index after the wrap instead of throwing.
Bug Report
For English only, other languages will not accept.
Which version of ShardingSphere did you use?
master (latest)
Which project did you use? ShardingSphere-JDBC or ShardingSphere-Proxy?
Both —
RoundRobinLoadBalanceAlgorithmlives ininfraand is the defaultROUND_ROBINload balancer used by readwrite-splitting.Expected behavior
RoundRobinLoadBalanceAlgorithm.getTargetName()keeps returning a valid target name for the lifetime of the process.Actual behavior
Once the internal counter passes
Integer.MAX_VALUE, every call throwsArrayIndexOutOfBoundsExceptionfor any replica group whose size is not a power of two.Reason analyze (If you can)
RoundRobinLoadBalanceAlgorithm.java:36:Math.abs(Integer.MIN_VALUE)isInteger.MIN_VALUE—abscannot represent2^31in anint, so it returns the negative value unchanged. The remainder is then negative andList.getthrows:Math.abs(Integer.MIN_VALUE) % sizePower-of-two group sizes happen to yield 0 and survive, which is why a 2-replica setup never shows it.
getTargetNameis annotated@HighFrequencyInvocationand the counter is never reset, so a long-lived proxy handling read traffic reaches the wrap on its own.There is prior art for this exact failure: #1265 reported the same
ArrayIndexOutOfBoundsExceptionin 2018 and was closed as completed. The fix at the time was acount.compareAndSet(readDataSourceNames.size(), 0)line that kept the counter bounded. That line was later removed in #17422 while the counter was being changed from a static map to an instance field, and theMath.abs(...)expression was kept. (For the record, that old line was itself racy under concurrency — several threads could step pastsizebefore any of them observed it — so the point is not that a good guard was lost, but that the current code has no bound on the counter at all.)Steps to reproduce the behavior
Set the counter to
Integer.MAX_VALUEand call the algorithm twice against a 3-element list:Example codes for reproduce this issue (such as a github link)
The snippet above runs as-is inside
RoundRobinLoadBalanceAlgorithmTest. I have a fix and a regression test ready and will open a PR referencing this issue.Math.floorMod(count.getAndIncrement(), availableTargetNames.size())is a one-line replacement that is identical to the current expression for every non-negative counter value — I verified that exhaustively for all counters in[0, 5_000_000]across group sizes 1..16 — and returns a valid index after the wrap instead of throwing.