roachtest: acceptance/bank/node-restart flake #57798
Labels
Comments
RaduBerinde
added
C-test-failure
|
Timed out here: cockroach/pkg/cmd/roachtest/bank.go Lines 56 to 64 in 3c09757 Unfortunately this will be annoying to track down. For some reason this also just happened on release-20.2: #57982 |
|
Ah, that one is helpful, thanks @ajwerner! Look at this: which is from the latest log of n1. The first stack proves that the binary was started 9 minutes ago. The second one shouldn't be there nine minutes in - it's a one-off synchronous write of the node status in Throughout these nine minutes, in the main logs (the stacks were in the stderr logs, I think roachtest sends a SIGSEGV on timeouts so that stacks are dumped) we see a number of "have been waiting": However - I never see any such messages from the raft level. Instead, in the goroutines, what I can see is concurrency control. So, I think a narrative starts to form:
@nvanbenschoten this could benefit from an extra pair of eyes (more precisely your eyes :-) ) Full artifacts here, The stacks are here: reporter tries to scan meta2, sits in distsender while pushing a lock
There are two more stacks with one in WaitOn
and another in waitOn and pushing (sitting in distsender)
I sure have some trouble here. Everyone's stuck in DistSender (btw, all of these stacks are on n3; didn't find anything in the other logs other than trouble writing the initial summary); otherwise, I should be seeing someone in Oh, hmm, I am seeing this: At the same time the replicaGCQueue is regularly timing out: |
pbardea
added
branch-release-20.2
release-blocker
|
I didn't get around to writing this down yesterday, but the issue is known now. We don't allow VOTER_{INCOMING,OUTGOING,DEMOTING} to acquire a lease, but we did add code that prevents non-raft leaders to acquire the lease (to fix #37906). So we end up in a place where n4 is VOTER_INCOMING and is raft leader, and n2 is asked to get a lease, and it refuses (redirecting requests to n4), but n4 cannot actually get a lease, ad infinitum. For cockroach/pkg/kv/kvserver/replica_raft.go Lines 264 to 272 in 89781b1 So if cockroach/pkg/kv/kvserver/replica_command.go Lines 1068 to 1098 in a8612df "Luckily", we don't use One prudent option to fix the bug without getting into too much of this mess is to allow lease acquisition if the raft leader looks like a VOTER_OUTGOING or VOTER_DEMOTING. If we don't have the leader in the descriptor (under its replicaID) then we're definitely behind - don't try to get a lease; send to the leader which in fact is very likely to be a full voter. If we do have it, and it's in either of these states, get the lease ourselves (or try). We could, in principle, be far behind and so this regresses partially on #37906, but we're not likely to hit this as readily as the original issues, if at all. Assigning to @andreimatei who will work on a fix. |
andreimatei
added a commit
to andreimatei/cockroach
that referenced
this issue
Jan 6, 2021
This reverts commit 8f98ade. This is a revert of the main commit in cockroachdb#57789 (which was a backport of cockroachdb#55148). It turns out that the commit in question introduced a deadlock: if replicas that are not the leader refuse to take the lease (because their not the leader) and the leader is a VOTER_INCOMING, VOTER_DEMOTING or VOTER_OUTGOING replica which also refuses to take the lease because it's not a VOTER_FULL [0] => deadlock. This deadlock was found in cockroachdb#57798. The patch will return with some massaging. This revert is original work on the 20.2 branch; I'm not reverting it on master in the hope of just fixing the issue. Touches cockroachdb#37906 Release note: This voids the previous release note reading "A bug causing queries sent to a freshly-restarted node to sometimes hang for a long time while the node catches up with replication has been fixed."
pbardea
removed
branch-release-20.2
release-blocker
This was referenced Jan 11, 2021
andreimatei
added a commit
to andreimatei/cockroach
that referenced
this issue
Jan 11, 2021
This patch backpedals a little bit on the logic introduced in cockroachdb#55148. That patch said that, if a leader is known, every other replica refuses to propose a lease acquisition. Instead, the replica in question redirects whomever was triggering the lease acquisition to the leader, thinking that the leader should take the lease. That patch introduced a deadlock: some replicas refuse to take the lease because they are not VOTER_FULL (see CheckCanReceiveLease()). To fix the deadlock, this patch incorporates that check in the proposal buffer's decision about whether or not to reject a proposal: if the leader is believed to refuse to take the lease, then we again forward our own lease request. An edge case to the edge case is when the leader is not even part of the proposer's range descriptor. This can happen if the proposer is far behind. In this case, we assume that the leader is eligible. If it isn't, the deadlock will resolve once the proposer catches up. A future patch will relax the conditions under which a replica agrees to take the lease. VOTER_INCOMING replicas should take the lease. VOTER_DEMOTING are more controversial. Fixes cockroachdb#57798 Release note: None
andreimatei
added a commit
to andreimatei/cockroach
that referenced
this issue
Jan 14, 2021
This patch backpedals a little bit on the logic introduced in cockroachdb#55148. That patch said that, if a leader is known, every other replica refuses to propose a lease acquisition. Instead, the replica in question redirects whomever was triggering the lease acquisition to the leader, thinking that the leader should take the lease. That patch introduced a deadlock: some replicas refuse to take the lease because they are not VOTER_FULL (see CheckCanReceiveLease()). To fix the deadlock, this patch incorporates that check in the proposal buffer's decision about whether or not to reject a proposal: if the leader is believed to refuse to take the lease, then we again forward our own lease request. An edge case to the edge case is when the leader is not even part of the proposer's range descriptor. This can happen if the proposer is far behind. In this case, we assume that the leader is eligible. If it isn't, the deadlock will resolve once the proposer catches up. A future patch will relax the conditions under which a replica agrees to take the lease. VOTER_INCOMING replicas should take the lease. VOTER_DEMOTING are more controversial. Fixes cockroachdb#57798 Release note: None
andreimatei
added a commit
to andreimatei/cockroach
that referenced
this issue
Jan 22, 2021
This patch backpedals a little bit on the logic introduced in cockroachdb#55148. That patch said that, if a leader is known, every other replica refuses to propose a lease acquisition. Instead, the replica in question redirects whomever was triggering the lease acquisition to the leader, thinking that the leader should take the lease. That patch introduced a deadlock: some replicas refuse to take the lease because they are not VOTER_FULL (see CheckCanReceiveLease()). To fix the deadlock, this patch incorporates that check in the proposal buffer's decision about whether or not to reject a proposal: if the leader is believed to refuse to take the lease, then we again forward our own lease request. An edge case to the edge case is when the leader is not even part of the proposer's range descriptor. This can happen if the proposer is far behind. In this case, we assume that the leader is eligible. If it isn't, the deadlock will resolve once the proposer catches up. A future patch will relax the conditions under which a replica agrees to take the lease. VOTER_INCOMING replicas should take the lease. VOTER_DEMOTING are more controversial. Fixes cockroachdb#57798 Release note: None
craig bot
pushed a commit
that referenced
this issue
Jan 25, 2021
58722: kvserver: don't refuse to fwd lease proposals in some edge cases r=andreimatei a=andreimatei This patch backpedals a little bit on the logic introduced in #55148. That patch said that, if a leader is known, every other replica refuses to propose a lease acquisition. Instead, the replica in question redirects whomever was triggering the lease acquisition to the leader, thinking that the leader should take the lease. That patch introduced a deadlock: some replicas refuse to take the lease because they are not VOTER_FULL (see CheckCanReceiveLease()). To fix the deadlock, this patch incorporates that check in the proposal buffer's decision about whether or not to reject a proposal: if the leader is believed to refuse to take the lease, then we again forward our own lease request. An edge case to the edge case is when the leader is not even part of the proposer's range descriptor. This can happen if the proposer is far behind. In this case, we assume that the leader is eligible. If it isn't, the deadlock will resolve once the proposer catches up. A future patch will relax the conditions under which a replica agrees to take the lease. VOTER_INCOMING replicas should take the lease. VOTER_DEMOTING are more controversial. Fixes #57798 Release note: None 59087: util/log: new output format 'crdb-v2' r=itsbilal a=knz Fixes #50166. This new format intends to address all the known shortcomings with `crdb-v1` while remaining compatible with entry parsers designed for the previous version. See the user-facing release note below for a summary of changes; and the included reference documentation for details. Example TTY output with colors:  Example for a single-line unstructured entry. Before: ``` I210116 22:17:03.736236 57 cli/start.go:681 ⋮ node startup completed: ``` After: ``` I210116 22:17:03.736236 57 cli/start.go:681 ⋮ [-] 12 node startup completed: tag field now always included ^^^ entry counter now always included ^^^ ``` Example for a multi-line unstructured entry. Before: ``` I210116 22:15:38.105666 452 gossip/gossip.go:567 ⋮ [n1] 74 gossip status (ok, 1 node‹›) gossip client (0/3 cur/max conns) gossip connectivity n1 [sentinel]; ``` (subsequent lines lack a log entry prefix; hard to determine where entries start and end) After: ``` I210116 22:15:38.105666 452 gossip/gossip.go:567 ⋮ [n1] 74 gossip status (ok, 1 node‹›) I210116 22:15:38.105666 452 gossip/gossip.go:567 ⋮ [n1] 74 +gossip client (0/3 cur/max conns) I210116 22:15:38.105666 452 gossip/gossip.go:567 ⋮ [n1] 74 +gossip connectivity I210116 22:15:38.105666 452 gossip/gossip.go:567 ⋮ [n1] 74 + n1 [sentinel]; ^^^ common prefix repeated for each msg line same entry counter for every subsequent line ^^^ continuation marker "+" on susequent lines ^^ ``` Example for a structured entry. Before: ``` I210116 22:14:38.175829 469 util/log/event_log.go:32 ⋮ [n1] Structured entry: {...} ``` After: ``` I210116 22:14:38.175829 469 util/log/event_log.go:32 ⋮ [n1] 21 ={...} entry counter always present ^^ equal sign "=" denotes structured entry ^^ ``` Release note (cli change): The default output format for `file-group` and `stderr` sinks has been changed to `crdb-v2`. This new format is non-ambiguous and makes it possible to reliably parse log files. Refer to the format's documentation for details. Additionally, it prevents single log lines from exceeding a large size; this problem is inherent to the `crdb-v1` format and can prevent `cockroach debug zip` from retrieving v1 log files. The new format has also been designed so that existinglog file analyzers for the `crdb-v1` format can read entries written the new format. However, this conversion may be imperfect. Again, refer to the new format's documentation for details. In case of incompatibility, users can force the previous format by using `format: crdb-v1` in their logging configuration. 59141: ui: upgrade admin-ui-components to new dep r=dhartunian a=dhartunian We renamed the `admin-ui-components` package to `cluster-ui`. Release note: None 59388: build,bazel: remove references to `gofmt` in bazel build r=rickystewart a=rickystewart This was cargo-culted from the `Makefile`, but isn't necessary to get the build to succeed, and interferes with hermiticity because it requires `gofmt` to be globally installed. It's simpler to just remove these references entirely. Release note: None Co-authored-by: Andrei Matei <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: David Hartunian <[email protected]> Co-authored-by: Ricky Stewart <[email protected]>
irfansharif
added a commit
to irfansharif/cockroach
that referenced
this issue
Feb 3, 2021
Fixes cockroachdb#57342. This looks to have been the same thing as cockroachdb#57798, and was fixed by cockroachdb#58722. Release note: None
|
@andreimatei and @tbg I'm afraid we may not be out of the woods just yet on this one. |
|
This test doesn't seem to have failed in a while. And I did 20 passing runs. Closing... |
andreimatei
added a commit
to andreimatei/cockroach
that referenced
this issue
Mar 22, 2021
This patch backpedals a little bit on the logic introduced in cockroachdb#55148. That patch said that, if a leader is known, every other replica refuses to propose a lease acquisition. Instead, the replica in question redirects whomever was triggering the lease acquisition to the leader, thinking that the leader should take the lease. That patch introduced a deadlock: some replicas refuse to take the lease because they are not VOTER_FULL (see CheckCanReceiveLease()). To fix the deadlock, this patch incorporates that check in the proposal buffer's decision about whether or not to reject a proposal: if the leader is believed to refuse to take the lease, then we again forward our own lease request. An edge case to the edge case is when the leader is not even part of the proposer's range descriptor. This can happen if the proposer is far behind. In this case, we assume that the leader is eligible. If it isn't, the deadlock will resolve once the proposer catches up. A future patch will relax the conditions under which a replica agrees to take the lease. VOTER_INCOMING replicas should take the lease. VOTER_DEMOTING are more controversial. Fixes cockroachdb#57798 Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
https://teamcity.cockroachdb.com/viewLog.html?buildId=2510921&buildTypeId=Cockroach_UnitTests
The text was updated successfully, but these errors were encountered: