kfaraz commented on code in PR #19541:
URL: https://github.com/apache/druid/pull/19541#discussion_r3401564163
##########
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.");
Review Comment:
Can this method be abstract instead?
##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorSpec.java:
##########
@@ -139,4 +139,23 @@ default void merge(@Nullable SupervisorSpec existingSpec)
{
// No-op by default
}
+
+ /**
+ * Given the currently-running {@code old} spec, returns whether replacing
it with this spec
+ * requires the supervisor to be restarted (re-submitted). This is distinct
from asking whether
+ * the two specs are equal: implementations may return {@code false} for
differences that do not
+ * affect the running supervisor (for example, a {@code taskCount} change
while autoscaling is
+ * enabled, since that field is overridden at runtime).
+ * <p>
+ * Only consulted once the two specs are known to differ (see
+ * {@link SupervisorManager#shouldUpdateSupervisor}); the conservative
default treats any
+ * such difference as requiring a restart.
+ *
+ * @param old the currently-running supervisor spec
+ * @return true if the supervisor must be restarted to apply this spec
+ */
+ default boolean requireRestart(SupervisorSpec old)
Review Comment:
It would make more sense to invoke this method on the currently running
(i.e. old) supervisor spec and pass the new one as an argument, since the
question we are asking is "should we restart the current supervisor?".
This would also align this method to `validateSpecUpdateTo()`, which takes
the new proposed spec as an argument.
--
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]
