[
https://issues.apache.org/jira/browse/KUDU-3793?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gabriella Lotz updated KUDU-3793:
---------------------------------
Description:
While running auto_leader_rebalancer-test under TSAN, I hit this crash during
cluster shutdown:
{code:java}
F leader_election.cc:206] Check failed: has_responded_
@ kudu::consensus::LeaderElection::~LeaderElection()
@ ... kudu::rpc::Proxy::RefreshDnsAndEnqueueRequest()::$_0::~...
@ kudu::DnsResolver::RefreshAddressesAsync()::$_1::~...{code}
DnsResolver turns a peer's name into IP addresses. Its async methods
(ResolveAddressesAsync / RefreshAddressesAsync) don't do the lookup right away.
They package the work into a task, put it on a threadpool queue, and promise to
call the caller's callback once a worker runs it.
The bug is what happens on shutdown. When the threadpool is stopped, any task
still sitting on the queue that hasn't run yet is just deleted, not run.
Deleting the task also deletes the callback stored inside it, so that callback
is thrown away having never been called.
That breaks a caller that's waiting on the callback. In this case it's a
LeaderElection: it kicks off an async address lookup so it can send a vote RPC,
and its callback holds the only remaining reference keeping the election object
alive. When the callback is discarded instead of called, the election gets
destroyed without ever "responding," and its destructor's
DCHECK(has_responded_) (a rule that the decision callback must always fire
exactly once) aborts the process.
This only shows up during shutdown with a lookup still in flight, which is
common in tests that repeatedly start and stop clusters, and more so under TSAN
because it's slow enough that a lookup is often still pending at teardown.
Since it's a DCHECK, it surfaces in debug/TSAN builds.
The root cause is in DnsResolver: the callback contract should be "always
invoked exactly once, even on failure," the way the RPC layer already
guarantees, but the async resolve path can silently drop it.
h3. Proposed fix
Make the async resolve methods guarantee the callback runs exactly once:
normally when the task runs, or with an error (e.g. ServiceUnavailable) if the
task is dropped before running. Then a caller like LeaderElection is notified
("the lookup was abandoned") and cleans up normally instead of tripping the
check.
This touches every async DNS resolution in the server, not just leader
elections, so it should get its own review and a unit test (submit a resolve,
shut the pool down, assert the callback fired with an error).
was:
While running auto_leader_rebalancer-test under TSAN, I hit this crash during
cluster shutdown:
{code:java}
F leader_election.cc:206] Check failed: has_responded_
@ kudu::consensus::LeaderElection::~LeaderElection()
@ ... kudu::rpc::Proxy::RefreshDnsAndEnqueueRequest()::$_0::~...
@ kudu::DnsResolver::RefreshAddressesAsync()::$_1::~...{code}
> LeaderElection hits has_responded_ CHECK on shutdown when async DNS callback
> is dropped
> ---------------------------------------------------------------------------------------
>
> Key: KUDU-3793
> URL: https://issues.apache.org/jira/browse/KUDU-3793
> Project: Kudu
> Issue Type: Bug
> Reporter: Gabriella Lotz
> Priority: Major
>
> While running auto_leader_rebalancer-test under TSAN, I hit this crash during
> cluster shutdown:
> {code:java}
> F leader_election.cc:206] Check failed: has_responded_
> @ kudu::consensus::LeaderElection::~LeaderElection()
> @ ... kudu::rpc::Proxy::RefreshDnsAndEnqueueRequest()::$_0::~...
> @ kudu::DnsResolver::RefreshAddressesAsync()::$_1::~...{code}
> DnsResolver turns a peer's name into IP addresses. Its async methods
> (ResolveAddressesAsync / RefreshAddressesAsync) don't do the lookup right
> away. They package the work into a task, put it on a threadpool queue, and
> promise to call the caller's callback once a worker runs it.
> The bug is what happens on shutdown. When the threadpool is stopped, any task
> still sitting on the queue that hasn't run yet is just deleted, not run.
> Deleting the task also deletes the callback stored inside it, so that
> callback is thrown away having never been called.
> That breaks a caller that's waiting on the callback. In this case it's a
> LeaderElection: it kicks off an async address lookup so it can send a vote
> RPC, and its callback holds the only remaining reference keeping the election
> object alive. When the callback is discarded instead of called, the election
> gets destroyed without ever "responding," and its destructor's
> DCHECK(has_responded_) (a rule that the decision callback must always fire
> exactly once) aborts the process.
> This only shows up during shutdown with a lookup still in flight, which is
> common in tests that repeatedly start and stop clusters, and more so under
> TSAN because it's slow enough that a lookup is often still pending at
> teardown. Since it's a DCHECK, it surfaces in debug/TSAN builds.
> The root cause is in DnsResolver: the callback contract should be "always
> invoked exactly once, even on failure," the way the RPC layer already
> guarantees, but the async resolve path can silently drop it.
> h3. Proposed fix
> Make the async resolve methods guarantee the callback runs exactly once:
> normally when the task runs, or with an error (e.g. ServiceUnavailable) if
> the task is dropped before running. Then a caller like LeaderElection is
> notified ("the lookup was abandoned") and cleans up normally instead of
> tripping the check.
> This touches every async DNS resolution in the server, not just leader
> elections, so it should get its own review and a unit test (submit a resolve,
> shut the pool down, assert the callback fired with an error).
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
