kfaraz commented on code in PR #18082:
URL: https://github.com/apache/druid/pull/18082#discussion_r2146402841
##########
extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/RabbitStreamIndexTask.java:
##########
@@ -48,21 +50,25 @@ public class RabbitStreamIndexTask extends
SeekableStreamIndexTask<String, Long,
@JsonCreator
public RabbitStreamIndexTask(
@JsonProperty("id") String id,
+ @JsonProperty("supervisorId") @Nullable String supervisorId,
@JsonProperty("resource") TaskResource taskResource,
@JsonProperty("dataSchema") DataSchema dataSchema,
@JsonProperty("tuningConfig") RabbitStreamIndexTaskTuningConfig
tuningConfig,
@JsonProperty("ioConfig") RabbitStreamIndexTaskIOConfig ioConfig,
@JsonProperty("context") Map<String, Object> context,
- @JacksonInject ObjectMapper configMapper)
+ @JacksonInject ObjectMapper configMapper
+ )
{
super(
getOrMakeId(id, dataSchema.getDataSource(), TYPE),
+ supervisorId,
taskResource,
dataSchema,
tuningConfig,
ioConfig,
context,
- getFormattedGroupId(dataSchema.getDataSource(), TYPE));
+ getFormattedGroupId(Configs.valueOrDefault(supervisorId,
dataSchema.getDataSource()), TYPE)
Review Comment:
I wonder if some of this logic should also go into the super class.
Since now we are trying to determine the default in both the super and
sub-classes.
##########
extensions-contrib/materialized-view-maintenance/src/main/java/org/apache/druid/indexing/materializedview/MaterializedViewSupervisor.java:
##########
@@ -133,7 +133,7 @@ public void start()
DataSourceMetadata metadata =
metadataStorageCoordinator.retrieveDataSourceMetadata(dataSource);
if (null == metadata) {
metadataStorageCoordinator.insertDataSourceMetadata(
- dataSource,
+ supervisorId,
Review Comment:
We should continue to use `dataSource` for `MaterializedViewSupervisor`.
This `supervisorId` field is only the thread name/logging key.
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorSpec.java:
##########
@@ -77,7 +79,33 @@ private static SeekableStreamSupervisorIngestionSpec
checkIngestionSchema(
private final boolean suspended;
protected final SupervisorStateManagerConfig supervisorStateManagerConfig;
+ /**
+ * Base constructor for SeekableStreamSupervisors.
+ *
+ * @param id The unique identifier for the
supervisor. If this parameter is null,
+ * the constructor will use the
non-null `ingestionSchema` to provide
+ * a default `dataSource` value as
the supervisor ID. This ensures that
+ * every supervisor has a non-null
ID, either user-specified or defaulting to
+ * the target `dataSource`.
+ * @param ingestionSchema The ingestion schema that
defines the configuration
+ * for the supervisor.
+ * @param context A map of additional context
parameters.
+ * @param suspended Indicates whether the supervisor
is initially
+ * suspended.
+ * @param taskStorage The storage for tasks.
+ * @param taskMaster The task master responsible for
task management.
+ * @param indexerMetadataStorageCoordinator The coordinator for indexer
+ * metadata storage.
+ * @param indexTaskClientFactory The factory for creating index
task clients.
+ * @param mapper The ObjectMapper for JSON
serialization and deserialization.
+ * @param emitter The service emitter for metrics
and alerts.
+ * @param monitorSchedulerConfig The configuration for the
monitor scheduler.
+ * @param rowIngestionMetersFactory The factory for creating row
ingestion meters.
+ * @param supervisorStateManagerConfig The configuration for the
supervisor
+ * state manager.
Review Comment:
I think we can get rid of these. Only the `id` param has any useful info.
--
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]
