Repository navigation
Description
Summary
When leader election is enabled, Operator#installShutdownHook(Duration) does not install any JVM shutdown hook. It only logs a warning and returns. This silently drops a behavior the caller explicitly opted into, and the rationale for doing so is neither documented in the code nor in the related commit history. On bare-JVM deployments this leaves the leader lease unreleased on SIGTERM, forcing standbys to wait for lease expiry before they can take over.
Additionally, the method's Duration gracefulShutdownTimeout parameter has been dead since PR #2479. It is accepted but never honored at runtime, yet the JavaDoc still describes it as the timeout used during shutdown.
Code references
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java
// lines 143-150
@SuppressWarnings("unused")
public void installShutdownHook(Duration gracefulShutdownTimeout) {
if (!leaderElectionManager.isLeaderElectionEnabled()) {
Runtime.getRuntime().addShutdownHook(new Thread(this::stop));
} else {
log.warn("Leader election is on, shutdown hook will not be installed.");
}
}And the stop() method already handles leader election correctly:
// lines 191-214 (relevant excerpt)
@Override
public void stop() throws OperatorException {
...
controllerManager.stop();
configurationService.getExecutorServiceManager().stop(reconciliationTerminationTimeout);
leaderElectionManager.stop(); // <-- already releases the lease
if (configurationService.closeClientOnStop()) {
getKubernetesClient().close();
}
...
}Observations
1. The asymmetry between installShutdownHook and stop
stop() is fully aware of leader election: it calls leaderElectionManager.stop(). If the shutdown hook simply invoked this::stop (which it does in the non-leader-election branch), the lease would be released cleanly on JVM shutdown. There is no apparent technical reason the hook can't be installed when leader election is on as the cleanup path is already there.
The current behavior means that:
- The user explicitly calls
installShutdownHook(...)to request graceful shutdown on SIGTERM. - JOSDK silently declines to honor that request (only a
log.warn). - On
SIGTERM, the JVM exits without invokingstop(). leaderElectionManager.stop()never runs.- The lease held by the dying leader remains in etcd until its
leaseDurationelapses. - Standby replicas wait out the lease before electing themselves leader.
This is the opposite of what most users would expect from "install a graceful shutdown hook on a leader-elected operator".
2. Dead gracefulShutdownTimeout parameter
Originally installShutdownHook(Duration) called stop(gracefulShutdownTimeout), honoring the per-call value. Commit 9390d89e ("feat: support for graceful shutdown based on configuration", PR #2479) re-routed the timeout to come from ConfigurationService.reconciliationTerminationTimeout() and changed the hook to call the no-arg this::stop. The Duration parameter was kept in the signature (probably for binary compatibility) but is no longer used, hence the @SuppressWarnings("unused"). The JavaDoc still says:
@param gracefulShutdownTimeout timeout to wait for executor threads to complete actual reconciliations
…which is misleading. Callers can pass any value and it will have no effect.
3. No documented rationale for the skip
I searched the commit history for installShutdownHook and the warn-and-skip behavior, and could not find a commit message or PR description explaining why the hook is skipped when leader election is on. The closest hypothesis I can construct is that "frameworks like Spring/Quarkus install their own shutdown hooks, so JOSDK should not double-register", but that is never stated in the code, and it also leaves bare-JVM users with broken graceful shutdown.
Open Questions
- Is the skip-on-leader-election behavior intentional? If so, what's the underlying concern (double-hook registration with a framework, race with the lease renewer, something else)?
- Should
installShutdownHook(...)install the hook regardless of leader election, and rely onstop()(which already handlesleaderElectionManager.stop()) to do the right thing? - The
Durationparameter oninstallShutdownHook(Duration)is unused. Should it be deprecated in favor of a no-arg overload, with the JavaDoc updated to point atConfigurationServiceOverrider#withReconciliationTerminationTimeout(...)as the real configuration knob?
Possible directions
Depending on maintainer guidance, any of these are reasonable PR scopes (single PR or split):
- Docs-only: update the JavaDoc and the
log.warnmessage to explain the current behavior and point users at the right pattern for framework-managed shutdown. No behavior change. - Behavior fix: install the hook regardless of leader-election state, since
stop()is already leader-election-aware. - API cleanup: deprecate the
Durationparameter; introduceinstallShutdownHook()(no-arg) and update the JavaDoc to referencewithReconciliationTerminationTimeout(...).
Activity
csviri commentedon May 26, 2026
Thank you @Dennis-Mircea for reporting.
@xstefank @metacosm if you have the bandwidth , I'm quite budy with other stuff this week and next week I'm off.
metacosm commentedon May 27, 2026
@Dennis-Mircea skipping the hook registration when leader election is enabled apparently was added in #1618 to address a deadlock on shutdown. I'm actually not sure why this was done this way as I don't recall the details. I haven't been too involved with leader election but I think that it would certainly deserve an API clean-up because I see potential problems for sure, in particular with the registration of callbacks, which isn't particularly documented and might lead to issues since the framework needs to do its own processing in this situation. Since what the framework needs isn't documented, it's possible the user-registered callbacks might conflict with them.
Dennis-Mircea commentedon May 27, 2026
Thanks for the context, @metacosm! Pulled up #1618 and the diff confirms the skip was added to avoid a shutdown deadlock, but neither the PR description nor the merge commit explains what specifically deadlocked. @csviri, do you happen to remember (or have a reproducer for) the original symptom? If the underlying issue has since been addressed in the lifecycle code (e.g. by the executor service / leader manager refactors), we may now be able to re-enable the hook safely.
Separately, I'd appreciate guidance on what operators currently relying on
installShutdownHook(...)in an HA setup should do today. For example, Flink Kubernetes Operator has only this in place at startup:With leader election enabled (the whole point of HA), this line is effectively a no-op: the hook is silently not installed, the
Durationargument is ignored (dead since #2479), and onSIGTERMthe JVM exits without releasing the lease. Is the intended pattern that HA users calloperator.stop()from their own application lifecycle instead of from a JVM hook? If so, that's worth surfacing in the JavaDoc and probably in a short note in the leader-election docs as well.Either way, agreed the surface area deserves a clean-up. I can start with a docs-only PR (JavaDoc + log message + deprecation of the unused
Durationparameter oninstallShutdownHook), leaving any behavior change for a separate PR once we know more about the original deadlock and the recommended HA pattern.csviri commentedon May 27, 2026
Hi @Dennis-Mircea thank you to pointing those out. I think the point 2. was not cleaned up when that PR was merged, I would deprecate that method an marked for removal and document that
reconciliationTerminationTimeoutshould be used instead.will come back regarding 1. and 3., that we should document for sure; will check why we are not adding the shutdown hook in that case.
csviri commentedon May 27, 2026
So afaik this is the issue:
if there is a leader election in place, a pod is either a leader or not. If it is the leader but would lost lead it calls a hard
System.exit(...), the shutdown hook should not be present at this point, since we want graceful stop.This had the deadlock issue in the past:
#1614
If it's not the leader, and receives a SIGTERM, the event processing is not running, so we don't want to have a shutdown hook either.
However this is not entirely right this way, since if a pod is a leader and receives a SIGTERM but does not loose lead we still might want to have gracefull termination.
Dennis-Mircea commentedon May 27, 2026
Thanks @csviri, this clarifies things a lot. I pulled up #1614 to confirm the deadlock shape and the stack trace is very specific:
So the deadlock is not really "leader election + shutdown" in general. It's the specific path:
Operator.stop()Operator.stop()callsleaderElectionManager.stop(), which cancels the leader-election futureonStopLeadingcallbackLeaderElectionManager.stopLeading) callsSystem.exit(...)System.exittries to acquirejava.lang.Shutdown's lock, which is already held by the shutdown hook thread we're running on -> deadlockThe cure for the deadlock is therefore narrower than "skip the shutdown hook under leader election". It's: don't invoke
System.exitwhenstop()has already been called, because at that point we are intentionally tearing down rather than reacting to a lost lease. With this small improvement, the shutdown hook can be re-enabled unconditionally and case 3 (leader receives SIGTERM, still holds lead) is fixed for free.Concretely, the fix looks like a single boolean flag on
LeaderElectionManager:That preserves the "restart on lost lead" behavior (case 1) and unblocks both case 2 and case 3, without introducing a second leader-election cleanup code path.
Regarding the leader vs non-leader distinction you mentioned: as far as I can tell,
leaderElectionManager.stop()is already safe to call on a non-leader (no lease to release, the underlying Fabric8 elector handles it). So I don't think we need an explicitif (isLeader)branch inOperator.stop(). Installing the shutdown hook unconditionally and letting the existingstop()flow run for both leaders and non-leaders looks like the simplest correct behavior.Proposed scope for the follow-up PR (separate from the deprecation cleanup we already agreed on for point 2):
System.exit(...)inLeaderElectionManager.stopLeadingwhenstop()has already been called.Operator.installShutdownHook()so the hook is registered for every operator.Does this direction work for you? Do you want to make the deprecation part and the actual fix in the same PR or do you want to have separate PRs for both?
csviri commentedon May 27, 2026
sounds good! let's see the code :) thank you!