From d58b230153d209f870f110f6139a177e0952d277 Mon Sep 17 00:00:00 2001 From: Danny Faught Date: Fri, 27 Sep 2024 12:49:29 -0700 Subject: [PATCH] fix: overflow bug * Fixes https://issues.apache.org/jira/browse/CASSANDRA-14098 in a similar way as the provided patch. * Also refactored and added tests. Co-Authored-By: Lada Kesseler <23501754+lexler@users.noreply.github.com> --- .../cassandra/service/StorageService.java | 9 +++---- .../cassandra/service/StorageServiceTest.java | 26 ++++++++++++++----- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index e82cc48afa..46cd69340b 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -3556,11 +3556,10 @@ public class StorageService extends NotificationBroadcasterSupport implements IE static int calculateSplitCount(int keysPerSplit, long totalRowCountEstimate, int numberOfKeys) { int minSamplesPerSplit = 4; - int maxSplitCount = numberOfKeys / minSamplesPerSplit + 1; - int calculatedSplitCount = (int) (totalRowCountEstimate / keysPerSplit); - int splitCountMaxOrLess = Math.min(maxSplitCount, calculatedSplitCount); - int splitCountOneOrHigher = Math.max(1, splitCountMaxOrLess); - return splitCountOneOrHigher; + long maxSplitCount = numberOfKeys / minSamplesPerSplit + 1; + long calculatedSplitCount = totalRowCountEstimate / keysPerSplit; + int splitCountWithLimit = (int) Math.min(maxSplitCount, calculatedSplitCount); + return Math.max(1, splitCountWithLimit); } private List, Long>> getSplits(List tokens, int splitCount, ColumnFamilyStore cfs) diff --git a/test/unit/org/apache/cassandra/service/StorageServiceTest.java b/test/unit/org/apache/cassandra/service/StorageServiceTest.java index 7a403c01bf..33c9befb78 100644 --- a/test/unit/org/apache/cassandra/service/StorageServiceTest.java +++ b/test/unit/org/apache/cassandra/service/StorageServiceTest.java @@ -318,23 +318,37 @@ public class StorageServiceTest extends TestBaseImpl } @Test - public void calculateSplitCountHappyPath() + public void calculateSplitCount_SmallRowCount_NotLimited() { - int result = StorageService.calculateSplitCount(2, 4, 40); - assertEquals(2, result); + int result = StorageService.calculateSplitCount(2, 8, 40); + assertEquals(4, result); } @Test - public void calculateSplitCountForMin() + public void calculateSplitCount_ForLargeRowCount_LimitsResult() { int result = StorageService.calculateSplitCount(2, 100, 40); assertEquals(11, result); } @Test - public void calculateSplitCountForMaxWithOverflow() + public void calculateSplitCount_ForMaxIntegerRowCount_LimitsResult() + { + int result = StorageService.calculateSplitCount(1, Integer.MAX_VALUE, 4); + assertEquals(2, result); + } + + @Test + public void calculateSplitCount_ForOverflowingRowCount_LimitsResult() + { + int result = StorageService.calculateSplitCount(1, Integer.MAX_VALUE + 1L, 4); + assertEquals(2, result); + } + + @Test + public void calculateSplitCount_ForMaxLongRowCount_LimitsResult() { int result = StorageService.calculateSplitCount(1, Long.MAX_VALUE, 4); - assertEquals(1, result); + assertEquals(2, result); } }