From 3afd97eae02c14cce3a295ca0a2042631e25f8ee Mon Sep 17 00:00:00 2001 From: Dun Lu Date: Tue, 3 Oct 2023 20:17:27 -0700 Subject: [PATCH 1/4] fix typo --- .../druid/timeline/partition/OvershadowableManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java b/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java index 8d010cf43b67..7c154cdb355b 100644 --- a/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java +++ b/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java @@ -418,9 +418,9 @@ private Iterator>> stateMap ) { - final RootPartitionRange lowFench = new RootPartitionRange(partitionId, partitionId); + final RootPartitionRange lowFence = new RootPartitionRange(partitionId, partitionId); final RootPartitionRange highFence = new RootPartitionRange(Short.MAX_VALUE, Short.MAX_VALUE); - return stateMap.subMap(lowFench, false, highFence, false).entrySet().iterator(); + return stateMap.subMap(lowFence, false, highFence, false).entrySet().iterator(); } /** From 345c4b5a4e9acdb9893c8b044ac353c36d3ef1bc Mon Sep 17 00:00:00 2001 From: Dun Lu Date: Wed, 4 Oct 2023 10:47:57 -0700 Subject: [PATCH 2/4] handle out-of-range partitionIds --- .../partition/OvershadowableManager.java | 12 +++++++- .../partition/OvershadowableManagerTest.java | 30 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java b/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java index 7c154cdb355b..f4d020a0cacb 100644 --- a/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java +++ b/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java @@ -39,6 +39,7 @@ import it.unimi.dsi.fastutil.shorts.ShortSortedSets; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.Overshadowable; import javax.annotation.Nullable; @@ -85,6 +86,7 @@ enum State OVERSHADOWED } + private static final Logger log = new Logger(OvershadowableManager.class); private final Map> knownPartitionChunks; // served segments // (start partitionId, end partitionId) -> minorVersion -> atomicUpdateGroup @@ -418,7 +420,8 @@ private Iterator>> stateMap ) { - final RootPartitionRange lowFence = new RootPartitionRange(partitionId, partitionId); + final short partitionIdLowFence = partitionId < 0 ? Short.MAX_VALUE : partitionId; + final RootPartitionRange lowFence = new RootPartitionRange(partitionIdLowFence, partitionIdLowFence); final RootPartitionRange highFence = new RootPartitionRange(Short.MAX_VALUE, Short.MAX_VALUE); return stateMap.subMap(lowFence, false, highFence, false).entrySet().iterator(); } @@ -1054,6 +1057,13 @@ private static > RootPartitionRange of(AtomicUpdateG private RootPartitionRange(short startPartitionId, short endPartitionId) { + if (startPartitionId < 0 || endPartitionId < 0) { + log.error( + "PartitionId [%s],[%s] possibly out of range of Short.MAX_VALUE, please compact your segements or reduce number of segments in time peroid ", + startPartitionId, + endPartitionId + ); + } this.startPartitionId = startPartitionId; this.endPartitionId = endPartitionId; } diff --git a/processing/src/test/java/org/apache/druid/timeline/partition/OvershadowableManagerTest.java b/processing/src/test/java/org/apache/druid/timeline/partition/OvershadowableManagerTest.java index 92f0fed47b4a..440e8e0bb783 100644 --- a/processing/src/test/java/org/apache/druid/timeline/partition/OvershadowableManagerTest.java +++ b/processing/src/test/java/org/apache/druid/timeline/partition/OvershadowableManagerTest.java @@ -163,6 +163,36 @@ public void testFindOvershadowedBy() Collections.emptyList(), overshadowedGroups ); + + } + + @Test + public void testHandleOutOfRangeFindOvershadowedBy() + { + final List> expectedOvershadowedChunks = new ArrayList<>(); + PartitionChunk chunk = newNonRootChunk(32001, 32003, 1, 1); + manager.addChunk(chunk); + expectedOvershadowedChunks.add(chunk); + manager.addChunk(newNonRootChunk(32001, 32005, 2, 1)); + manager.addChunk(newNonRootChunk(32767, 32768, 3, 1)); + manager.addChunk(newNonRootChunk(32768, 32769, 3, 1)); + + List> overshadowedGroups = manager.findOvershadowedBy( + RootPartitionRange.of(32000, 32767), + (short) 4, + State.OVERSHADOWED + ); + Assert.assertEquals( + expectedOvershadowedChunks.stream().map(AtomicUpdateGroup::new).collect(Collectors.toList()), + overshadowedGroups + ); + + overshadowedGroups = manager.findOvershadowedBy( + RootPartitionRange.of(32769, 32769), + (short) 10, + State.VISIBLE + ); + Assert.assertTrue(overshadowedGroups.isEmpty()); } @Test From ef527f6890aeec55c3798f29129548159f50140a Mon Sep 17 00:00:00 2001 From: Dun Lu Date: Wed, 4 Oct 2023 10:52:08 -0700 Subject: [PATCH 3/4] format --- .../druid/timeline/partition/OvershadowableManagerTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/processing/src/test/java/org/apache/druid/timeline/partition/OvershadowableManagerTest.java b/processing/src/test/java/org/apache/druid/timeline/partition/OvershadowableManagerTest.java index 440e8e0bb783..6efc0329332c 100644 --- a/processing/src/test/java/org/apache/druid/timeline/partition/OvershadowableManagerTest.java +++ b/processing/src/test/java/org/apache/druid/timeline/partition/OvershadowableManagerTest.java @@ -163,7 +163,6 @@ public void testFindOvershadowedBy() Collections.emptyList(), overshadowedGroups ); - } @Test From 85b08b02ff1c8562e20628c075ccd5d5bb5ceb97 Mon Sep 17 00:00:00 2001 From: Dun Lu Date: Wed, 4 Oct 2023 13:59:19 -0700 Subject: [PATCH 4/4] add comments --- .../apache/druid/timeline/partition/OvershadowableManager.java | 1 + 1 file changed, 1 insertion(+) diff --git a/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java b/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java index f4d020a0cacb..86df6987d7b4 100644 --- a/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java +++ b/processing/src/main/java/org/apache/druid/timeline/partition/OvershadowableManager.java @@ -420,6 +420,7 @@ private Iterator>> stateMap ) { + // remediate submap `fromKey > toKey` issue when partitionId overflows final short partitionIdLowFence = partitionId < 0 ? Short.MAX_VALUE : partitionId; final RootPartitionRange lowFence = new RootPartitionRange(partitionIdLowFence, partitionIdLowFence); final RootPartitionRange highFence = new RootPartitionRange(Short.MAX_VALUE, Short.MAX_VALUE);