roachtest: failover/system-non-liveness/blackhole-recv failed #99931
Assignees
Labels
A-kv
Anything in KV that doesn't belong in a more specific category.
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
C-test-failure
Broken test (automatically or manually discovered).
O-roachtest
O-robot
Originated from a bot.
Milestone
Comments
cockroach-teamcity
added
branch-release-23.1
nvanbenschoten
removed
the
release-blocker
|
This was an instance of #98698. |
|
This is a failure in the side transport. Looking through the logs, the following happened. (the regression timestamps are 13:26:15.380 and 13:26:16.289; +3s is when they are likely issued by the side transport) Node 1 had moved the replicas to 4, 5, 6 16 minutes earlier: Node 5 was cut off from the network (with an async partition) ~30 sec earlier: It was still trying to transfer the lease over to n4 at this point - but I don't think it succeeded because of the network outage (although it thinks its for a different reason) My guess is that the lease was maybe ambiguous between n5 and n4 at this point in time. We cut off incoming connections to n5, but it still had outgoing connections so it might have sent a side transport update with a stale lease. |
It would be easy enough to log every incoming So we'd just need to update the test to run with |
|
I see a log on n5 right at the time this is happening: and also from n4 So n5 likely was sending a side-transport update to n6 when n6 crashed. I can tee up trying to reproduce this with the increased logging. |
pav-kv
added
C-bug
andrewbaptist
removed
the
branch-release-23.1
|
Removed the branch-release tag from this as it is not a release or GA blocker for 23.1 With the latest dialback changes this is less likely to happen on this test, but this is still a real bug in that we need to get to the bottom of. However, it has been happening for a long time. |
|
We're tracking this as a GA blocker over in #98698, @pavelkalinnikov will have a look this week. |
|
cc @cockroachdb/replication |
pav-kv
added a commit
to pav-kv/cockroach
that referenced
this issue
Apr 27, 2023
The PROSCRIBED lease status, just like EXPIRED, puts a lease to a definitely invalid state. The UNUSABLE state (when "now" is in stasis period) is less of a clear cut: some callers may treat it as invalid, and some may thinks it's still valid. For example, the closed timestamp side-transport ignores the UNUSABLE state, and takes it as valid. Because of the order in which the checks were made, this has lead to a bug: a PROSCRIBED lease is reported as UNUSABLE during stasis periods, the closed timestamp side-transport then considers it valid, and updates closed timestamps when it shouldn't. This commit fixes the bug by swapping the order of checks in the leaseStatus method. The order now goes from "hard" checks like EXPIRED and PROSCRIBED, to "softer" UNUSABLE, and (when the softness is put to the limit) VALID. Fixes cockroachdb#98698 Fixes cockroachdb#99931 Fixes cockroachdb#100101 Epic: none Release note (bug fix): a bug is fixed in closed timestamp updates within its side-transport. Previously, during asymmetric partitions, a node that transfers a lease away, and misses a liveness heartbeat, could then erroneously update the closed timestamp during the stasis period of its liveness. This could lead to closed timestamp invariant violation, and node crashes; in extreme cases this could lead to inconsistencies in read-only queries.
pav-kv
added a commit
to pav-kv/cockroach
that referenced
this issue
Apr 27, 2023
The PROSCRIBED lease status, just like EXPIRED, puts a lease to a definitely invalid state. The UNUSABLE state (when request timestamp is in stasis period) is less of a clear cut: some callers may treat it as invalid, and some may think it's still valid. For example, the closed timestamp side-transport ignores the UNUSABLE state, and takes it as valid. Because of the order in which the checks were made, this has lead to a bug: a PROSCRIBED lease is reported as UNUSABLE during stasis periods, the closed timestamp side-transport then considers it valid, and updates closed timestamps when it shouldn't. This commit fixes the bug by swapping the order of checks in the leaseStatus method. The order now goes from "hard" checks like EXPIRED and PROSCRIBED, to "softer" UNUSABLE, and (when the softness is put to the limit) VALID. Fixes cockroachdb#98698 Fixes cockroachdb#99931 Fixes cockroachdb#100101 Epic: none Release note (bug fix): a bug is fixed in closed timestamp updates within its side-transport. Previously, during asymmetric partitions, a node that transfers a lease away, and misses a liveness heartbeat, could then erroneously update the closed timestamp during the stasis period of its liveness. This could lead to closed timestamp invariant violation, and node crashes; in extreme cases this could lead to inconsistencies in read-only queries.
craig bot
pushed a commit
that referenced
this issue
Apr 28, 2023
100181: kv: Use strict types for common fields r=erikgrinaker a=andrewbaptist This PR introduces 3 new typed fields in mvcc.go: RaftTerm, RaftIndex and LeaseSequence. These fields were previously just unit64 throughout the code and this made the code harder to read and risked incorrect conversions. Epic: none Release note: None 102407: kvserver: check PROSCRIBED lease status over UNUSABLE r=erikgrinaker,tbg a=pavelkalinnikov The PROSCRIBED lease status, just like EXPIRED, puts a lease to a definitely invalid state. The UNUSABLE state (when request timestamp is in stasis period) is less of a clear cut: we still own the lease but callers may use or not use it depending on context. For example, the closed timestamp side-transport ignores the UNUSABLE state (because we still own the lease), and takes it as usable for its purposes. Because of the order in which the checks were made, this has lead to a bug: a PROSCRIBED lease is reported as UNUSABLE during stasis periods, the closed timestamp side-transport then considers it usable, and updates closed timestamps when it shouldn't. This commit fixes the bug by swapping the order of checks in the leaseStatus method. The order now goes from "hard" checks like EXPIRED and PROSCRIBED, to "softer" UNUSABLE, and (when the softness is put to the limit) VALID. Fixes #98698 Fixes #99931 Fixes #100101 Epic: none Release note (bug fix): a bug is fixed in closed timestamp updates within its side-transport. Previously, during asymmetric partitions, a node that transfers a lease away, and misses a liveness heartbeat, could then erroneously update the closed timestamp during the stasis period of its liveness. This could lead to closed timestamp invariant violation, and node crashes; in extreme cases this could lead to inconsistencies in read-only queries. 102503: concurrency: do not partition locks in the lock table by span scope r=nvanbenschoten a=arulajmani This patch is entirely a refactor and does not change any functionality. This is done in preparation for introducing `LockSpanSets` to track lock spans, which do not make a distinction between global and local keys (unlike `SpanSets`, which do). The main changes here are in `lockTableImpl`, which actually stores locks, and `lockTableGuardImpl` which snapshots the lock table. We no longer make a distinction between locks on Local and Global keys when storing them. The majority of this diff is composed of test file churn caused because of the printing changes to the lock table. Informs #102008 Release note: None 102590: sql,persistedsqlstats: prevent a deadlock during shutdown r=j82w a=knz Fixes #102574. Prior to this change, the coordination between the stats flusher task (an async stopper task) and the activity flusher job was performed using a two-step process: - the stats persistence task offered to call a callback _function_ every time a flush would complete. - the job would _reconfigure the callback function_ on each iteration. - the function was writing to a channel that was subsequently read by the job iteration body. This approach was defective in 3 ways: 1. if the job iteration body would exit (e.g. due to a server drain) *after* it installed the callback fn, but *before* the stats flusher would read and call the callback fn, a window of time existed where a deadlock could occur: - the stats flusher retrieves the pointer to the caller fn but doesn't call it yet. - the job loop exits. From then on it will not read from the channel any more. - the stats flusher attempts to write to the channel. A deadlock occurs. (This was seen during testing. See #102574) The fix here is to always jointly `select` the write to the channel and also a read from the drain/stopper signals, to abort the channel operation if a shutdown is requested. 2. the stats flusher task was holding the mutex locked while performing the channel write. This is generally bad code hygiene as it forces the code maintainer to double-check whether the lock and channel operations don't mutually interlock. The fix is to use the mutex to retrieve the channel reference, and then write to the channel while the mutex is not held any more. 3. the API between the two was defining a *callback function* where really just a notification channel was needed. The fix here is to simplify the API. Release note: None Co-authored-by: Andrew Baptist <[email protected]> Co-authored-by: Pavel Kalinnikov <[email protected]> Co-authored-by: Arul Ajmani <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>
blathers-crl bot
pushed a commit
that referenced
this issue
Apr 28, 2023
The PROSCRIBED lease status, just like EXPIRED, puts a lease to a definitely invalid state. The UNUSABLE state (when request timestamp is in stasis period) is less of a clear cut: we still own the lease but callers may use or not use it depending on context. For example, the closed timestamp side-transport ignores the UNUSABLE state (because we still own the lease), and takes it as usable for its purposes. Because of the order in which the checks were made, this has lead to a bug: a PROSCRIBED lease is reported as UNUSABLE during stasis periods, the closed timestamp side-transport then considers it usable, and updates closed timestamps when it shouldn't. This commit fixes the bug by swapping the order of checks in the leaseStatus method. The order now goes from "hard" checks like EXPIRED and PROSCRIBED, to "softer" UNUSABLE, and (when the softness is put to the limit) VALID. Fixes #98698 Fixes #99931 Fixes #100101 Epic: none Release note (bug fix): a bug is fixed in closed timestamp updates within its side-transport. Previously, during asymmetric partitions, a node that transfers a lease away, and misses a liveness heartbeat, could then erroneously update the closed timestamp during the stasis period of its liveness. This could lead to closed timestamp invariant violation, and node crashes; in extreme cases this could lead to inconsistencies in read-only queries.
blathers-crl bot
pushed a commit
that referenced
this issue
Apr 28, 2023
The PROSCRIBED lease status, just like EXPIRED, puts a lease to a definitely invalid state. The UNUSABLE state (when request timestamp is in stasis period) is less of a clear cut: we still own the lease but callers may use or not use it depending on context. For example, the closed timestamp side-transport ignores the UNUSABLE state (because we still own the lease), and takes it as usable for its purposes. Because of the order in which the checks were made, this has lead to a bug: a PROSCRIBED lease is reported as UNUSABLE during stasis periods, the closed timestamp side-transport then considers it usable, and updates closed timestamps when it shouldn't. This commit fixes the bug by swapping the order of checks in the leaseStatus method. The order now goes from "hard" checks like EXPIRED and PROSCRIBED, to "softer" UNUSABLE, and (when the softness is put to the limit) VALID. Fixes #98698 Fixes #99931 Fixes #100101 Epic: none Release note (bug fix): a bug is fixed in closed timestamp updates within its side-transport. Previously, during asymmetric partitions, a node that transfers a lease away, and misses a liveness heartbeat, could then erroneously update the closed timestamp during the stasis period of its liveness. This could lead to closed timestamp invariant violation, and node crashes; in extreme cases this could lead to inconsistencies in read-only queries.
blathers-crl bot
pushed a commit
that referenced
this issue
Apr 28, 2023
The PROSCRIBED lease status, just like EXPIRED, puts a lease to a definitely invalid state. The UNUSABLE state (when request timestamp is in stasis period) is less of a clear cut: we still own the lease but callers may use or not use it depending on context. For example, the closed timestamp side-transport ignores the UNUSABLE state (because we still own the lease), and takes it as usable for its purposes. Because of the order in which the checks were made, this has lead to a bug: a PROSCRIBED lease is reported as UNUSABLE during stasis periods, the closed timestamp side-transport then considers it usable, and updates closed timestamps when it shouldn't. This commit fixes the bug by swapping the order of checks in the leaseStatus method. The order now goes from "hard" checks like EXPIRED and PROSCRIBED, to "softer" UNUSABLE, and (when the softness is put to the limit) VALID. Fixes #98698 Fixes #99931 Fixes #100101 Epic: none Release note (bug fix): a bug is fixed in closed timestamp updates within its side-transport. Previously, during asymmetric partitions, a node that transfers a lease away, and misses a liveness heartbeat, could then erroneously update the closed timestamp during the stasis period of its liveness. This could lead to closed timestamp invariant violation, and node crashes; in extreme cases this could lead to inconsistencies in read-only queries.
blathers-crl bot
pushed a commit
that referenced
this issue
Apr 28, 2023
The PROSCRIBED lease status, just like EXPIRED, puts a lease to a definitely invalid state. The UNUSABLE state (when request timestamp is in stasis period) is less of a clear cut: we still own the lease but callers may use or not use it depending on context. For example, the closed timestamp side-transport ignores the UNUSABLE state (because we still own the lease), and takes it as usable for its purposes. Because of the order in which the checks were made, this has lead to a bug: a PROSCRIBED lease is reported as UNUSABLE during stasis periods, the closed timestamp side-transport then considers it usable, and updates closed timestamps when it shouldn't. This commit fixes the bug by swapping the order of checks in the leaseStatus method. The order now goes from "hard" checks like EXPIRED and PROSCRIBED, to "softer" UNUSABLE, and (when the softness is put to the limit) VALID. Fixes #98698 Fixes #99931 Fixes #100101 Epic: none Release note (bug fix): a bug is fixed in closed timestamp updates within its side-transport. Previously, during asymmetric partitions, a node that transfers a lease away, and misses a liveness heartbeat, could then erroneously update the closed timestamp during the stasis period of its liveness. This could lead to closed timestamp invariant violation, and node crashes; in extreme cases this could lead to inconsistencies in read-only queries.
cameronnunez
pushed a commit
to cameronnunez/cockroach
that referenced
this issue
May 2, 2023
The PROSCRIBED lease status, just like EXPIRED, puts a lease to a definitely invalid state. The UNUSABLE state (when request timestamp is in stasis period) is less of a clear cut: we still own the lease but callers may use or not use it depending on context. For example, the closed timestamp side-transport ignores the UNUSABLE state (because we still own the lease), and takes it as usable for its purposes. Because of the order in which the checks were made, this has lead to a bug: a PROSCRIBED lease is reported as UNUSABLE during stasis periods, the closed timestamp side-transport then considers it usable, and updates closed timestamps when it shouldn't. This commit fixes the bug by swapping the order of checks in the leaseStatus method. The order now goes from "hard" checks like EXPIRED and PROSCRIBED, to "softer" UNUSABLE, and (when the softness is put to the limit) VALID. Fixes cockroachdb#98698 Fixes cockroachdb#99931 Fixes cockroachdb#100101 Epic: none Release note (bug fix): a bug is fixed in closed timestamp updates within its side-transport. Previously, during asymmetric partitions, a node that transfers a lease away, and misses a liveness heartbeat, could then erroneously update the closed timestamp during the stasis period of its liveness. This could lead to closed timestamp invariant violation, and node crashes; in extreme cases this could lead to inconsistencies in read-only queries.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
roachtest.failover/system-non-liveness/blackhole-recv failed with artifacts on release-23.1 @ a34af06d8e7230e4052db80d55da21a06013f81b:
Parameters:
ROACHTEST_cloud=gce,ROACHTEST_cpu=4,ROACHTEST_encrypted=false,ROACHTEST_fs=ext4,ROACHTEST_localSSD=true,ROACHTEST_ssd=0Help
See: roachtest README
See: How To Investigate (internal)
This test on roachdash | Improve this report!
Jira issue: CRDB-26147
The text was updated successfully, but these errors were encountered: