HIVE-29720: Abort the service if Metastore cannot participate in the election#6621
HIVE-29720: Abort the service if Metastore cannot participate in the election#6621arunk-kumar wants to merge 5 commits into
Conversation
…election Retry exhaustion in LeaseLeaderElection previously terminated the election daemon thread with an uncaught RuntimeException and no UncaughtExceptionHandler, leaving HMS running with no leader for the HOUSEKEEPING or ALWAYS_TASKS category. Install an UncaughtExceptionHandler that logs the failing election and calls an injectable abort action (System.exit(1) in production, which triggers the existing HMS shutdown hooks). An AtomicBoolean ensures the abort runs at most once when both electors fail concurrently. Also adds @category(MetastoreUnitTest.class) to TestLeaderElection so its tests are picked up by the module's test.groups filter.
Extract the election-daemon UncaughtExceptionHandler into newAbortOnElectionFailureHandler() to drop start()'s cognitive complexity below the 15 threshold, and narrow the abortAction catch from Throwable to Exception. Also rewords a test comment that SonarCloud misidentified as commented-out code.
asf-ci-hive
added
tests pending
tests passed
and removed
tests unstable
tests pending
labels
Always start each election candidate in its own daemon thread so the UncaughtExceptionHandler fires on retry exhaustion regardless of how LeaderElectionContext is constructed. The startAsDaemon flag and its builder setter are removed since no caller sets it to true in production and the synchronous run() path bypassed the abort handler. Also updates HiveMetaStore's error message on election setup failure to reflect the new behavior (setup failure vs. runtime failure).
|
Thanks for the review! Pushed a commit addressing both points:
|
Per review feedback: - Default abortAction now calls ExitUtil.terminate(1) instead of System.exit(1), matching the pattern established by HIVE-29056 for JDK 21 compatibility. - The abort handler now best-effort closes the registered electors before invoking abortAction, so the mutex/lease is released promptly and other HMS instances can proceed with election. - close() iterates a snapshot of leaderElections to avoid concurrent modification if a close listener touches the context.
|
Thanks @dengzhhu653 — both addressed in the latest commit:
|
asf-ci-hive
added
tests pending
tests unstable
and removed
tests unstable
tests pending
labels
|
Sounds like the CI failures are related, e.g, TestMetastoreHousekeepingLeaderEmptyConfig, can you please take a look? Thank you! |
The switch from daemon.run() to daemon.start() in a previous commit broke a pre-existing invariant that LeaderElectionContext.start() would not return until listeners had fired synchronously. Fixing TestMetastoreHousekeepingLeaderEmptyConfig, which scans for housekeeping threads immediately after HMS starts. Fix: join each daemon after starting all of them. Elections still run in parallel (all daemons are started before the first join), but start() waits until every initial tryBeLeader() completes. LeaseLeaderElection.tryBeLeader returns after doWork() dispatches to a separate Heartbeater/NonLeaderWatcher, so join() terminates in bounded time in every case.
|
Thanks for the stack trace — that pinpointed it. The switch from Fix: join each daemon after starting all of them, so elections still run in parallel but
|
|
3 participants
What changes were proposed in this pull request?
Install an
UncaughtExceptionHandleron each election daemon inLeaderElectionContext.start(). On retry exhaustion it logs the failing election atERRORand calls an injectableabortAction(defaultSystem.exit(1)), which triggers the existing HMS shutdown-hook chain (electionclose(), ZooKeeper deregistration, metrics, servlet server). AnAtomicBooleanguards against a second abort when both electors fail concurrently. Also adds@Category(MetastoreUnitTest.class)toTestLeaderElection— the class was previously excluded by the module'stest.groupsfilter.Why are the changes needed?
Retry exhaustion in
LeaseLeaderElectionpreviously terminated the daemon thread with noUncaughtExceptionHandler— no log, no audit, no shutdown. HMS kept running with no leader forHOUSEKEEPING/ALWAYS_TASKS, silently stalling compaction and stats updates until users noticed via query slowdowns.Does this PR introduce any user-facing change?
Yes. When leader election exhausts metastore.lock.numretries retries, HMS now shuts down cleanly instead of remaining up without a leader for its housekeeping / always-tasks work. The existing retry knobs remain the tolerance dial.
How was this patch tested?
Added two unit tests to
TestLeaderElectionthat inject a failingLeaderElectionand a fakeabortAction, asserting the handler fires and that concurrent failures still trigger the abort exactly once. Full class runs in ~4.5s:Tests run: 4, Failures: 0, Errors: 0, Skipped: 0.