ZOOKEEPER-4946: Fix Login renewal thread not exiting on shutdown - #2446
IvanKhanas wants to merge 1 commit into
Conversation
161394b to
4dcefdb
Compare
|
Both precommit runs landed on the same agent and died the same way. Build #1 lost the agent almost immediately. Build #2 got through the whole suite first, I pushed a rebase onto current master to get another run. If it lands on |
There was a problem hiding this comment.
For example, when there is a failure in kerberos, the reLogin method call throws an exception. After retry has been set to 0, the reLogin method will be repeatedly called until successful.
Sounds like it is a dead loop bug but not interruption check things. No matter how frequently the check, we cann't interrupt Shell.execCommand and others. retry >= 0 seems to be always true.
4dcefdb to
0e084d2
Compare
0e084d2 to
3168e2a
Compare
Thanks for reading this so carefully. I was chasing how the interrupt got lost Fixed in 3168e2a. Once the retries run out, the loop breaks back into the |
anmolnar
left a comment
There was a problem hiding this comment.
Could you please document the config setting?
3168e2a to
cdadcfa
Compare
Done in cdadcfa. The entry is in the admin guide next to the other kerberos.* settings, plus a shorter one in the client properties list, since Login also runs on the client. |
After the reLogin() retries ran out, the retry loop never exited. When the KDC times out instead of refusing connections, each failed login takes longer than minReLoginTimeMs, so the thread called reLogin() forever and never checked for the interrupt from Login.shutdown(). shutdown() joined it without a timeout, and SendThread calls shutdown() on exit, so ZooKeeper.close() hung too. The retry loop now falls back to the renewal loop. Shell restores the interrupt status it used to swallow, and the join is bounded by zookeeper.kerberos.shutdownTimeoutMs.
cdadcfa to
92a00e8
Compare
Problem
The
reLogin()retry loop inLoginnever exited once the retries ran out: theelsebranch only logged, andretry >= 0stayed true.hasSufficientTimeElapsed()skips an attempt only if the previous one started less thanminReLoginTimeMsago (60s by default). When the KDC drops packets instead of refusing connections, the JDK waits 30s per try and retries 3 times, so every failed login takes longer than that. The thread then callsreLogin()forever and never reaches a sleep or an interrupt check.Login.shutdown()interrupted it and joined without a timeout.SendThreadcallsLogin.shutdown()on exit andClientCnxn.close()joinsSendThread, soZooKeeper.close()hung.NIOServerCnxnFactoryandNettyServerCnxnFactoryalso calllogin.shutdown().The interrupt could also be lost in
Shell.runCommand(), which caughtInterruptedExceptionwithout restoring the interrupt status. This only affects thekinitpath used with a ticket cache.Fix
reLogin()has already logged out, so no TGT is left and the loop takes the existing "No TGT found" branch: it sleepsminReLoginTimeMsand tries again. Renewal recovers when the KDC comes back.while (!Thread.currentThread().isInterrupted()).Shellrestores the interrupt status in bothcatch (InterruptedException)blocks.Login.shutdown()bounds the join withzookeeper.kerberos.shutdownTimeoutMs(5s by default) and logs a warning if the thread is still alive. Alogin.login()call in progress cannot be interrupted; the thread is a daemon.Tests
KerberosTicketRenewalTest.shouldLeaveRetryLoopWhenReLoginKeepsFailinglets everyreLogin()attempt run, as with a slow KDC, and checks that the thread leaves the retry loop.KerberosTicketRenewalTest.shouldNotBlockForeverWhenRenewalThreadDoesNotExitblocks the renewal thread in a call that ignores interrupts and checks thatshutdown()still returns.ShellTest(new) interrupts a thread insideShell.execCommandand checks that the interrupt status survives.Each test fails without its production change.