kfaraz commented on code in PR #19541:
URL: https://github.com/apache/druid/pull/19541#discussion_r3401521193


##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/LagAggregator.java:
##########
@@ -61,5 +61,19 @@ public <PartitionIdType> LagStats 
aggregate(Map<PartitionIdType, Long> partition
       final long avgLag = partitionLags.isEmpty() ? 0 : totalLag / 
partitionLags.size();
       return new LagStats(maxLag, totalLag, avgLag);
     }
+
+    // Stateless: all instances are equal. Needed because Jackson creates 
fresh instances on

Review Comment:
   Alternatively, you can make the constructor private and add a static 
`@JsonCreator` method which just returns the `DEFAULT` instance. 



##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorSpec.java:
##########
@@ -303,4 +377,114 @@ public abstract SeekableStreamSupervisorSpec 
createBackfillSpec(
       @Nullable Integer taskCount
   );
 
+  /**
+   * Returns a builder pre-populated with this spec's values (including 
injected services), so callers
+   * can produce a modified copy without mutating this instance. Subclasses 
return their own builder.
+   */
+  public Builder<?> toBuilder()
+  {
+    throw new UnsupportedOperationException("No builder is available for this 
supervisor spec.");
+  }
+
+  /**
+   * Self-typed builder for {@link SeekableStreamSupervisorSpec} and its 
subclasses. Holds the spec's
+   * components and injected services; subclasses implement {@link #self()} 
and {@link #build()} to
+   * reconstruct the concrete spec. Setter methods cover fields commonly 
copied or adjusted by callers.
+   */
+  @SuppressFBWarnings(
+      value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD",
+      justification = "Fields are populated via copyFrom() and read by build() 
in concrete subclasses, which "
+                      + "live in other modules and so are invisible to 
SpotBugs' per-module analysis."
+  )
+  public abstract static class Builder<T extends Builder<T>>
+  {
+    protected String id;
+    protected DataSchema dataSchema;
+    protected SeekableStreamSupervisorIOConfig ioConfig;
+    protected SeekableStreamSupervisorTuningConfig tuningConfig;
+    protected Map<String, Object> context;
+    protected Boolean suspended;
+    protected TaskStorage taskStorage;
+    protected TaskMaster taskMaster;
+    protected IndexerMetadataStorageCoordinator 
indexerMetadataStorageCoordinator;
+    protected SeekableStreamIndexTaskClientFactory indexTaskClientFactory;
+    protected ObjectMapper mapper;
+    protected ServiceEmitter emitter;
+    protected DruidMonitorSchedulerConfig monitorSchedulerConfig;
+    protected RowIngestionMetersFactory rowIngestionMetersFactory;
+    protected SupervisorStateManagerConfig supervisorStateManagerConfig;
+
+    protected abstract T self();
+
+    public abstract SeekableStreamSupervisorSpec build();
+
+    /**
+     * Copies all fields (components and injected services) from an existing 
spec into this builder.
+     */
+    public T copyFrom(SeekableStreamSupervisorSpec spec)
+    {
+      this.id = spec.id;
+      this.dataSchema = spec.getSpec().getDataSchema();
+      this.ioConfig = spec.getIoConfig();
+      this.tuningConfig = spec.getTuningConfig();
+      this.context = spec.context;
+      this.suspended = spec.suspended;
+      this.taskStorage = spec.taskStorage;
+      this.taskMaster = spec.taskMaster;
+      this.indexerMetadataStorageCoordinator = 
spec.indexerMetadataStorageCoordinator;
+      this.indexTaskClientFactory = spec.indexTaskClientFactory;
+      this.mapper = spec.mapper;
+      this.emitter = spec.emitter;
+      this.monitorSchedulerConfig = spec.monitorSchedulerConfig;
+      this.rowIngestionMetersFactory = spec.rowIngestionMetersFactory;
+      this.supervisorStateManagerConfig = spec.supervisorStateManagerConfig;

Review Comment:
   Will the builder ever use these dependencies?
   The output of the `build()` method will only ever be used for an equality 
check which should not involve the services anyway. So the `build()` method 
could simply pass these as null, atleast for now.



-- 
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]

Reply via email to