kfaraz commented on code in PR #16758: URL: https://github.com/apache/druid/pull/16758#discussion_r1684142201
########## docs/development/overview.md: ########## @@ -53,8 +53,17 @@ Most of the coordination logic for (real-time) ingestion is in the Druid indexin ## Real-time Ingestion -Druid loads data through `FirehoseFactory.java` classes. Firehoses often wrap other firehoses, where, similar to the design of the -query runners, each firehose adds a layer of logic, and the persist and hand-off logic is in `RealtimePlumber.java`. +Druid streaming tasks are based the 'seekable stream' classes such as `SeekableStreamSupervisor.java`, +`SeekableStreamIndexTask.java`, and `SeekableStreamIndexTaskRunner.java`. The data processing happen through Review Comment: ```suggestion `SeekableStreamIndexTask.java`, and `SeekableStreamIndexTaskRunner.java`. The data processing happens through ``` ########## docs/development/overview.md: ########## @@ -53,8 +53,17 @@ Most of the coordination logic for (real-time) ingestion is in the Druid indexin ## Real-time Ingestion -Druid loads data through `FirehoseFactory.java` classes. Firehoses often wrap other firehoses, where, similar to the design of the -query runners, each firehose adds a layer of logic, and the persist and hand-off logic is in `RealtimePlumber.java`. +Druid streaming tasks are based the 'seekable stream' classes such as `SeekableStreamSupervisor.java`, Review Comment: ```suggestion Druid streaming tasks are based on the 'seekable stream' classes such as `SeekableStreamSupervisor.java`, ``` ########## indexing-hadoop/src/main/java/org/apache/druid/indexer/hadoop/DatasourceRecordReader.java: ########## @@ -163,4 +189,157 @@ public void close() throws IOException FileUtils.deleteDirectory(dir); } } + + public static class SegmentReader implements Closeable Review Comment: I think it would be easier to review this if we leave this class in a separate file and just rename `IngestSegmentFirehose` to `SegmentReader`. ########## indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java: ########## @@ -152,8 +152,7 @@ default int getPriority() * the task does not use any. Users can be given permission to access particular types of * input sources but not others, using the * {@link org.apache.druid.server.security.AuthConfig#enableInputSourceSecurity} config. - * @throws UnsupportedOperationException if the given task type does not suppoert input source based security. Such - * would be the case, if the task uses firehose. + * @throws UnsupportedOperationException if the given task type does not suppoert input source based security Review Comment: I guess now it will never throw this exception. ########## indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java: ########## @@ -1155,18 +1139,9 @@ public IndexIOConfig( // old constructor for backward compatibility @Deprecated - public IndexIOConfig(FirehoseFactory firehoseFactory, @Nullable Boolean appendToExisting, @Nullable Boolean dropExisting) - { - this(firehoseFactory, null, null, appendToExisting, dropExisting); - } - - @Nullable - @JsonProperty("firehose") - @JsonInclude(Include.NON_NULL) - @Deprecated - public FirehoseFactory getFirehoseFactory() + public IndexIOConfig(@Nullable Boolean appendToExisting, @Nullable Boolean dropExisting) Review Comment: Do we still need this constructor? ########## indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexIOConfig.java: ########## @@ -37,26 +36,25 @@ public class ParallelIndexIOConfig extends IndexIOConfig { @JsonCreator public ParallelIndexIOConfig( - @JsonProperty("firehose") @Nullable FirehoseFactory firehoseFactory, @JsonProperty("inputSource") @Nullable InputSource inputSource, @JsonProperty("inputFormat") @Nullable InputFormat inputFormat, @JsonProperty("appendToExisting") @Nullable Boolean appendToExisting, @JsonProperty("dropExisting") @Nullable Boolean dropExisting ) { - super(firehoseFactory, inputSource, inputFormat, appendToExisting, dropExisting); + super(inputSource, inputFormat, appendToExisting, dropExisting); } // old constructor for backward compatibility @Deprecated - public ParallelIndexIOConfig(FirehoseFactory firehoseFactory, @Nullable Boolean appendToExisting) + public ParallelIndexIOConfig(@Nullable Boolean appendToExisting) { - this(firehoseFactory, null, null, appendToExisting, null); + this(null, null, appendToExisting, null); } @Deprecated - public ParallelIndexIOConfig(FirehoseFactory firehoseFactory, @Nullable Boolean appendToExisting, boolean dropExisting) + public ParallelIndexIOConfig(@Nullable Boolean appendToExisting, boolean dropExisting) Review Comment: We can completely remove these constructors now. ########## docs/operations/migrate-from-firehose-ingestion.md: ########## @@ -23,9 +23,7 @@ sidebar_label: "Migrate from firehose" ~ under the License. --> -Apache deprecated support for Druid firehoses in version 0.17. Support for firehose ingestion will be removed in version 26.0. - -If you're using a firehose for batch ingestion, we strongly recommend that you follow the instructions on this page to transition to using native batch ingestion input sources as soon as possible. +Apache deprecated support for Druid firehoses in version 0.17. Support for firehose ingestion were removed in version 26.0. Review Comment: ```suggestion Apache deprecated support for Druid firehoses in version 0.17. Support for firehose ingestion was removed in version 26.0. ``` -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org