FrankChen021 commented on code in PR #19271:
URL: https://github.com/apache/druid/pull/19271#discussion_r3499366721
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -1347,6 +1460,11 @@ public void tryInit()
try {
recordSupplier = setupRecordSupplier();
+ // Initialize bounded partitions BEFORE first run
+ if (ioConfig.isBounded()) {
Review Comment:
[P1] Bounded-mode calls do not compile
This introduces calls to `ioConfig.isBounded()` and
`ioConfig.getBoundedStreamConfig()` plus references to `BoundedStreamConfig`,
but `SeekableStreamSupervisorIOConfig` does not define those methods and no
`BoundedStreamConfig` type exists in the worktree. The indexing-service module
will fail to compile as soon as this file is built; add the config/type and
accessors, or remove these bounded-mode paths.
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -4692,6 +5180,25 @@ protected abstract
List<SeekableStreamIndexTask<PartitionIdType, SequenceOffsetT
@Nullable List<Integer> serverPrioritiesToAssign
) throws JsonProcessingException;
+ /**
+ * Converts a string representation of a partition ID to the typed partition
ID.
+ * Used for deserializing bounded stream config where partition keys come as
strings.
+ *
+ * @param partitionIdString string representation of partition ID
+ * @return typed partition ID
+ */
+ protected abstract PartitionIdType createPartitionIdFromString(String
partitionIdString);
Review Comment:
[P1] New abstract hooks are not implemented
The base class now declares `createPartitionIdFromString`,
`createSequenceOffsetFromObject`, `isOffsetAtOrBeyond`, and
`isEndOffsetExclusive` as abstract, but the concrete Kafka/Kinesis supervisors
and the seekable-stream test supervisors do not implement them. Those classes
will no longer compile unless these hooks get default implementations or all
subclasses are updated.
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -3156,11 +3311,64 @@ private boolean updatePartitionDataFromStream()
/**
Review Comment:
[P1] Public hook changes break protected overrides
This changes `getLatestSequencesFromStream` from protected to public;
similar public changes were made to `updatePartitionLagFromStream`,
`getOffsetsFromMetadataStorage`, and `createDataSourceMetaDataForReset`.
Existing overrides in Kafka, Kinesis, and test supervisors are still protected,
which Java rejects as reducing visibility. Keep these hooks protected or update
every override to public.
--
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]
