-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kv/kvnemesis: TestKVNemesisMultiNode failed #111893
Comments
Informs cockroachdb#111352. Informs cockroachdb#111530. Informs cockroachdb#111564. Informs cockroachdb#111893. This commit demonstrates that when an initial call to the lockTable's ScanAndEnqueue method returns an error, the request was not previously being dequeued from any other wait-queues that it may have entered. Release note: None
Informs cockroachdb#111352. Informs cockroachdb#111530. Informs cockroachdb#111564. Informs cockroachdb#111893. In 8205b43, we added a case to the lock table where a request's initial scan could throw an error. This was not being handled properly if the request had already entered any other lock wait-queues. In these cases, the request's entries in those wait-queues would be abandoned and the locks would get stuck. This commit fixes that issue by dequeuing the request from the lock table when throwing an error from ScanAndEnqueue. This was one of the causes of the recent kvnemesis instability, but we believe that there is at least one other issue that is still causing timeouts. Release note: None
111884: cast: remove invalid casts for REFCURSOR r=DrewKimball a=DrewKimball #### cast: remove invalid casts for REFCURSOR The initial implementation for the `REFCURSOR` data type included assignment casts from and explicit casts to every other type. In postgres, `REFCURSOR` has only the following valid casts: 1. Explicit casts from each string type. 2. Assignment casts to each string type. This patch aligns the casts for `REFCURSOR` with those of postgres, and adds according tests. Informs #111560 Release note: None 111923: kv: dequeue request from lock table wait-queues on scan error r=nvanbenschoten a=nvanbenschoten Informs #111352. Informs #111530. Informs #111564. Informs #111893. In 8205b43, we added a case to the lock table where a request's initial scan could throw an error. This was not being handled properly if the request had already entered any other lock wait-queues. In these cases, the request's entries in those wait-queues would be abandoned and the locks would get stuck. This commit fixes that issue by dequeuing the request from the lock table when throwing an error from ScanAndEnqueue. This was one of the causes of the recent kvnemesis instability, but we believe that there is at least one other issue that is still causing timeouts. Release note: None Co-authored-by: Drew Kimball <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]>
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ 981b050f28f406b495b530ec5725fa2c91cd20c5:
Parameters: |
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ 330500d0433bae42bc7c0d6842a427aab3b27f03:
Parameters: |
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ 330500d0433bae42bc7c0d6842a427aab3b27f03:
Parameters: |
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ bee7cfe930cabbbee3c813a273872e785e3b8ef4:
Parameters: |
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ bee7cfe930cabbbee3c813a273872e785e3b8ef4:
Parameters: |
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ 619aa6edad0269f75d12cb7d4b88b52f09825c7b:
Parameters: |
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ 8637692f77d69cda6052fef8e063cdac53870d3c:
Parameters: Same failure on other branches
|
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ fbbf7ef7869c725a73663f13347d38a413b33abd:
Parameters: Same failure on other branches
|
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ a45a76f76ee5d7bf87f5e642410d9be497caa051:
Parameters: Same failure on other branches
|
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ a45a76f76ee5d7bf87f5e642410d9be497caa051:
Parameters: Same failure on other branches
|
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ 9ae8d5cd9eae8df71dd7128f2cd02fea1f9d5fec:
Parameters: Same failure on other branches
|
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ 9ae8d5cd9eae8df71dd7128f2cd02fea1f9d5fec:
Parameters: Same failure on other branches
|
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ e945e0dfa7aa644fec63d2550acbf137d1f3f77b:
Parameters: Same failure on other branches
|
Informs cockroachdb#112221. Informs cockroachdb#112174. Informs cockroachdb#112173. Informs cockroachdb#111984. Informs cockroachdb#111893. Informs cockroachdb#111564. Informs cockroachdb#111530. This commit fixes the handling of replayed batches that contain a replicated lock acquisition and a later write to the same key. In such cases, the write at the higher sequence number should not be detected as an error during the replay. Instead, it should simply be ignored. Release note: None
112725: sql: disallow refcursor comparisons during type-checking r=DrewKimball a=DrewKimball This patch adds back the comparison functions for the `REFCURSOR` data type, since there are various points within the codebase where we rely on their existence. Users are still not allowed to use the comparison functions, but now this is checked during type-checking of the AST, rather than at execution-time. This should avoid internal errors from places in planning and execution that expect comparison overloads to exist, but also maintains parity with postgres. Fixes #112365 Fixes #112642 Fixes #112362 Fixes #112368 Release note: None 112891: sql: fix for crash with null elements on jsonb_array_to_string_array r=maryliag a=maryliag Fixes #112829 The builtin `jsonb_array_to_string_array` was crashing, when it had `null` elements. This commit handles this case, removing them from the final array. Release note: None 112893: kv: tolerate lock acquisition replay with writes in same batch r=arulajmani a=nvanbenschoten Informs #112221. Informs #112174. Informs #112173. Informs #111984. Informs #111893. Informs #111564. Informs #111530. This commit fixes the handling of replayed batches that contain a replicated lock acquisition and a later write to the same key. In such cases, the write at the higher sequence number should not be detected as an error during the replay. Instead, it should simply be ignored. Release note: None Co-authored-by: Drew Kimball <[email protected]> Co-authored-by: maryliag <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]>
Informs #112221. Informs #112174. Informs #112173. Informs #111984. Informs #111893. Informs #111564. Informs #111530. This commit fixes the handling of replayed batches that contain a replicated lock acquisition and a later write to the same key. In such cases, the write at the higher sequence number should not be detected as an error during the replay. Instead, it should simply be ignored. Release note: None
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ aabfe2e105d1633d110050c236c8e0d8c107e168:
Parameters: Same failure on other branches
|
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ c933595a44fe594ccb1aa1c553315a2dae902767:
Parameters: Same failure on other branches
|
Informs cockroachdb#111984. Informs cockroachdb#111893. Informs cockroachdb#111530. This commit changes storage.CheckSSTConflicts to also check for replicated locks, in addition to checking for intents. This is necessary to prevent AddSSTable from ingesting key-values pairs below replicated locks, which could violate the isolation expected from these locks if an ingested key-value is below the commit timestamp of the lock holder. This was caught in kvnemesis by the validation logic recently introduced in 7ff1c79. The change alters the semantics of CheckSSTConflicts slightly for intents. It now detects conflicts with any intents in the AddSSTable's key span, instead of just those on overlapping keys. I think this is ok, as the `checkConflicts = false` path in `EvalAddSSTable` already had this behavior. This is reflected in the changes to cmd_add_sstable_test.go, where three cases are no longer inconsistent with blind writes. I also don't think this is a performance concern (like the comment that's being deleted was suggesting) because we now have the separated lock table. In fact, this change allows us to avoid using an `intentInterleavingIter`, so if anything, it may provide a small performance win. Release note: None
Informs cockroachdb#111984. Informs cockroachdb#111893. Informs cockroachdb#111530. This commit changes storage.CheckSSTConflicts to also check for replicated locks, in addition to checking for intents. This is necessary to prevent AddSSTable from ingesting key-values pairs below replicated locks, which could violate the isolation expected from these locks if an ingested key-value is below the commit timestamp of the lock holder. This was caught in kvnemesis by the validation logic recently introduced in 7ff1c79. The change alters the semantics of CheckSSTConflicts slightly for intents. It now detects conflicts with any intents in the AddSSTable's key span, instead of just those on overlapping keys. I think this is ok, as the `checkConflicts = false` path in `EvalAddSSTable` already had this behavior. This is reflected in the changes to cmd_add_sstable_test.go, where three cases are no longer inconsistent with blind writes. I also don't think this is a performance concern (like the comment that's being deleted was suggesting) because we now have the separated lock table. In fact, this change allows us to avoid using an `intentInterleavingIter`, so if anything, it may provide a small performance win. Release note: None
113058: roachtest: add min support version for 24.1 in multitenant-upgrade r=yuzefovich a=yuzefovich Fixes: #112776. Release note: None 113120: storage: check for replicated locks in CheckSSTConflicts r=nvanbenschoten a=nvanbenschoten Informs #111984. Informs #111893. Informs #111530. This commit changes storage.CheckSSTConflicts to also check for replicated locks, in addition to checking for intents. This is necessary to prevent AddSSTable from ingesting key-values pairs below replicated locks, which could violate the isolation expected from these locks if an ingested key-value is below the commit timestamp of the lock holder. This was caught in kvnemesis by the validation logic recently introduced in 7ff1c79. The change alters the semantics of CheckSSTConflicts slightly for intents. It now detects conflicts with any intents in the AddSSTable's key span, instead of just those on overlapping keys. I think this is ok, as the `checkConflicts = false` path in `EvalAddSSTable` already had this behavior. This is reflected in the changes to cmd_add_sstable_test.go, where three cases are no longer inconsistent with blind writes. I also don't think this is a performance concern (like the comment that's being deleted was suggesting) because we now have the separated lock table. In fact, this change allows us to avoid using an `intentInterleavingIter`, so if anything, it may provide a small performance win. Release note: None Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]>
Informs cockroachdb#111984. Informs cockroachdb#111893. Informs cockroachdb#111530. This commit changes storage.CheckSSTConflicts to also check for replicated locks, in addition to checking for intents. This is necessary to prevent AddSSTable from ingesting key-values pairs below replicated locks, which could violate the isolation expected from these locks if an ingested key-value is below the commit timestamp of the lock holder. This was caught in kvnemesis by the validation logic recently introduced in 7ff1c79. The change alters the semantics of CheckSSTConflicts slightly for intents. It now detects conflicts with any intents in the AddSSTable's key span, instead of just those on overlapping keys. I think this is ok, as the `checkConflicts = false` path in `EvalAddSSTable` already had this behavior. This is reflected in the changes to cmd_add_sstable_test.go, where three cases are no longer inconsistent with blind writes. I also don't think this is a performance concern (like the comment that's being deleted was suggesting) because we now have the separated lock table. In fact, this change allows us to avoid using an `intentInterleavingIter`, so if anything, it may provide a small performance win. Release note: None
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ aa85c69776da942b4d539891725b780ba995401e:
Parameters: Same failure on other branches
|
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ cedd409f2115655efe2c529f6052f3056673b277:
Parameters: Same failure on other branches
|
This comment was marked as duplicate.
This comment was marked as duplicate.
Informs cockroachdb#111893, and others. Deflakes kvnemesis, until cockroachdb#113768 is addressed. Release note: None
kv/kvnemesis.TestKVNemesisMultiNode failed with artifacts on master @ b28fa0dbc55332c4bf01e8dd8399b827e9944672:
Fatal error:
Stack:
Log preceding fatal error
Help
See also: How To Investigate a Go Test Failure (internal)
This test on roachdash | Improve this report!
Jira issue: CRDB-32119
The text was updated successfully, but these errors were encountered: