jihoonson commented on a change in pull request #10033:
URL: https://github.com/apache/druid/pull/10033#discussion_r445310128
##########
File path:
core/src/test/java/org/apache/druid/timeline/partition/HashBasedNumberedPartialShardSpecTest.java
##########
@@ -73,5 +73,21 @@ public void testJsonPropertyNames() throws IOException
Assert.assertEquals(expected.getPartitionDimensions(),
map.get("partitionDimensions"));
Assert.assertEquals(expected.getBucketId(), map.get("bucketId"));
Assert.assertEquals(expected.getNumBuckets(), map.get("numPartitions"));
+ Assert.assertEquals(expected.getBucketId(), map.get("bucketId"));
+ }
+
+ @Test
+ public void testComplete()
+ {
+ final HashBasedNumberedPartialShardSpec partialShardSpec = new
HashBasedNumberedPartialShardSpec(
+ ImmutableList.of("dim"),
+ 2,
+ 4
+ );
+ final ShardSpec shardSpec = partialShardSpec.complete(new ObjectMapper(),
1, 3);
Review comment:
Ah, I didn't notice it while copying these tests 😅
##########
File path:
indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/RangePartitionMultiPhaseParallelIndexingTest.java
##########
@@ -201,48 +208,122 @@ private static void writeRow(
public void createsCorrectRangePartitions() throws Exception
{
int targetRowsPerSegment = NUM_ROW / DIM_FILE_CARDINALITY / NUM_PARTITION;
- final Set<DataSegment> publishedSegments;
+ final Set<DataSegment> publishedSegments = runTestTask(
+ new SingleDimensionPartitionsSpec(
+ targetRowsPerSegment,
+ null,
+ DIM1,
+ false
+ ),
+ useMultivalueDim ? TaskState.FAILED : TaskState.SUCCESS,
+ false
+ );
+
+ if (!useMultivalueDim) {
+ assertRangePartitions(publishedSegments);
+ }
+ }
+
+ @Test
+ public void
testAppendLinearlyPartitionedSegmensToHashPartitionedDatasourceSuccessfullyAppend()
Review comment:
Thanks 👍
##########
File path:
server/src/test/java/org/apache/druid/server/shard/SingleDimensionShardSpecTest.java
##########
@@ -142,6 +146,16 @@ public void testPossibleInDomain()
Assert.assertTrue(shard7.possibleInDomain(domain2));
}
+ @Test
+ public void testSharePartitionSpace()
+ {
+ final SingleDimensionShardSpec shardSpec = makeSpec("start", "end");
+
Assert.assertTrue(shardSpec.sharePartitionSpace(NumberedPartialShardSpec.instance()));
+ Assert.assertTrue(shardSpec.sharePartitionSpace(new
HashBasedNumberedPartialShardSpec(null, 0, 1)));
+ Assert.assertTrue(shardSpec.sharePartitionSpace(new
SingleDimensionPartialShardSpec("dim", 0, null, null, 1)));
Review comment:
Your understanding is correct. I added
`HashBasedNumberedPartialShardSpec` and `SingleDimensionPartialShardSpec` in
#7547 but they are not in use yet. They will be used once we support append
with non-dynamic or non-dynamic with segment locking.
##########
File path: core/src/main/java/org/apache/druid/timeline/partition/ShardSpec.java
##########
@@ -125,8 +125,13 @@ default short getAtomicUpdateGroupSize()
boolean possibleInDomain(Map<String, RangeSet<String>> domain);
/**
- * Returns true if two segments of this and other shardSpecs can exist in
the same time chunk.
+ * Returns true if this shardSpec and the given {@link PartialShardSpec}
share the same partition space.
+ * Any implementation of {@link OverwriteShardSpec} should return true.
+ *
+ * @see PartitionIds
*/
- @JsonIgnore
- boolean isCompatible(Class<? extends ShardSpec> other);
+ default boolean sharePartitionSpace(PartialShardSpec partialShardSpec)
+ {
+ return !partialShardSpec.useNonRootGenerationPartitionSpace();
Review comment:
Ah, all `ShardSpec`s share the same root-generation partition space
except `OverwriteShardSpec`. So, the current code is correct. I updated the
javadoc to make it more clear.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
