jon-wei commented on a change in pull request #10288:
URL: https://github.com/apache/druid/pull/10288#discussion_r493033048
##########
File path:
core/src/main/java/org/apache/druid/timeline/partition/HashBasedNumberedShardSpec.java
##########
@@ -298,8 +211,73 @@ private boolean chunkPossibleInDomain(
return false;
}
- private static int getBucketIndex(int hash, int numBuckets)
+ /**
+ * Check if the current segment possibly holds records if the values of
dimensions in {@link #partitionDimensions}
+ * are of {@code partitionDimensionsValues}
+ *
+ * @param hashPartitionFunction hash function used to create segments at
ingestion time
+ * @param partitionDimensionsValues An instance of values of dimensions in
{@link #partitionDimensions}
+ *
+ * @return Whether the current segment possibly holds records for the given
values of partition dimensions
+ */
+ private boolean isInChunk(HashPartitionFunction hashPartitionFunction,
Map<String, String> partitionDimensionsValues)
{
- return Math.abs(hash % numBuckets);
+ assert !partitionDimensions.isEmpty();
+ List<Object> groupKey = Lists.transform(
+ partitionDimensions,
+ o -> Collections.singletonList(partitionDimensionsValues.get(o))
+ );
+ return hashPartitionFunction.hash(serializeGroupKey(jsonMapper, groupKey),
numBuckets) == bucketId;
+ }
+
+ /**
+ * Serializes a group key into a byte array. The serialization algorithm can
affect hash values of partition keys
+ * since {@link HashPartitionFunction#hash} takes the result of this method
as its input. This means, the returned
+ * byte array should be backwards-compatible in cases where we need to
modify this method.
Review comment:
I wonder if we'll eventually need a second versioning scheme (like
"serialization version"). It doesn't seem needed for now since we only have
primitive + String dimension types and the serialization doesn't seem likely to
change there.
If we had "complex" dimensions at some point, I could see that being
difficult to support backwards compatibility for if the serialization format of
a complex dim changed.
----------------------------------------------------------------
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]
