FrankChen021 commented on code in PR #19568:
URL: https://github.com/apache/druid/pull/19568#discussion_r3380361877
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java:
##########
@@ -615,12 +625,10 @@ private SupervisorSpec
possiblyStopAndRemoveSupervisorInternal(String id, boolea
}
if (writeTombstone) {
- metadataSupervisorManager.insert(
- id,
- new NoopSupervisorSpec(null, pair.rhs.getDataSources())
- ); // where NoopSupervisorSpec is a tombstone
+ // NoopSupervisorSpec is a tombstone
+ metadataSupervisorManager.insert(id, new NoopSupervisorSpec(null,
pair.rhs.getDataSources()));
}
- pair.lhs.stop(true);
+ pair.lhs.stop(pair.lhs.stopGracefullyOnNewSpec());
Review Comment:
[P2] Do not use the new-spec stop policy for terminate
This helper is also called by stopAndRemoveSupervisor(id) with
writeTombstone=true, so explicit supervisor terminate now passes
stopGracefullyOnNewSpec() into stop(). The new default returns false, and an
implementation may return false specifically to avoid killing tasks during spec
replacement; on the public terminate path that means stop(false), which the
Supervisor contract says leaves running tasks as-is even though the terminate
API promises to stop associated tasks and publish segments. Please keep
tombstone/terminate removal graceful, or split the replacement policy from the
removal policy.
--
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]
