FrankChen021 commented on code in PR #19689:
URL: https://github.com/apache/druid/pull/19689#discussion_r3595300584
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/StreamingPartitionsSpec.java:
##########
@@ -19,104 +19,39 @@
package org.apache.druid.indexing.seekablestream;
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonInclude;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import org.apache.druid.error.DruidException;
+import com.fasterxml.jackson.annotation.JsonSubTypes;
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
import javax.annotation.Nullable;
-import java.util.Collections;
-import java.util.List;
-import java.util.Objects;
/**
* Streaming analog of the batch/compaction {@code partitionsSpec}. Configured
in the streaming tuning config, it
- * declares the dimensions whose observed values each published segment should
record in a
- * {@link org.apache.druid.timeline.partition.DimensionValueSetShardSpec} so
the broker can prune segments at query time
+ * declares <em>what</em> a streaming task should collect from its rows so
each published segment can be stamped with a
+ * prunable {@link org.apache.druid.timeline.partition.ShardSpec}, letting the
broker prune segments at query time
* without waiting for compaction. Unlike batch partitioning this does not
route rows into shards; it only annotates
- * segments with the values they happened to ingest.
+ * segments based on the values they happened to ingest.
*
- * <p>Use low-to-medium cardinality dimensions; the {@code
partitionDimensions} here should be kept in sync with the
- * {@code partitionDimensions} of the compaction config for the same
datasource.
+ * <p>This is a polymorphic, pluggable type (mirroring {@link
org.apache.druid.indexer.partitions.PartitionsSpec}): each
+ * implementation pairs a per-task {@link StreamingShardSpecCollector} (the
"collect something per row, then build a
+ * shard spec" operation) with the shard-spec type it produces. The built-in
{@link DimensionValueSetPartitionsSpec}
+ * collects the distinct values of a set of dimensions; future strategies
(e.g. min/max ranges or bloom filters) can be
+ * added by registering a new subtype below with its own collector, without
changing the task runner.
+ *
+ * <p>For backward compatibility the type defaults to {@link
DimensionValueSetPartitionsSpec}, so existing configs that
+ * specify only {@code partitionDimensions} (no {@code type}) continue to
deserialize unchanged.
*/
-public class StreamingPartitionsSpec
+@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl =
DimensionValueSetPartitionsSpec.class)
Review Comment:
[P2] Reject unknown streaming partition types
`defaultImpl` is used by Jackson for unrecognized type IDs as well as
omitted IDs. Consequently, an explicit typo such as
`{"type":"dim_value_sets"}`, or a future subtype reaching a peon that has not
loaded its extension, silently deserializes as
`DimensionValueSetPartitionsSpec`; unknown fields are globally ignored, so the
task may run the built-in collector or disable collection instead of rejecting
the unsupported strategy. This can hide rolling-deployment or plugin mistakes
and publish a different shard-spec strategy than requested. Preserve the
missing-type fallback but add Druid's
`@JsonTypeResolver(StrictTypeIdResolver.Builder.class)` so explicit unknown IDs
fail.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
