kfaraz commented on code in PR #19700:
URL: https://github.com/apache/druid/pull/19700#discussion_r3613953109
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java:
##########
@@ -197,7 +197,16 @@ public SupervisorSpecUpdateResult
createOrUpdateAndStartSupervisor(
if (!skipRestartIfUnmodified) {
// Always stop/recreate, persisting whenever the spec actually changed
(or is new).
final boolean specChanged = isSpecChangedAndValidated(spec);
- final SupervisorSpec existingSpec =
possiblyStopAndRemoveSupervisorInternal(spec.getId(), false);
+ final Pair<Supervisor, SupervisorSpec> current =
supervisors.get(spec.getId());
Review Comment:
Yeah, I was torn between the two options myself.
But then I realized the restart here really refers to the task restart
itself, since that is where the churn happens. Restarting the supervisor itself
is a cheap operation.
So when `skipRestart` is false, user doesn't want to skip restart at all, so
we should restart both tasks and supervisors.
As for the default behaviour, we can update the default value of this flag
in `SupervisorResource` in the next release (Druid 39).
--
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]
