-
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/kvserver: TestSystemZoneConfigs failed #98200
Comments
cockroach/pkg/kv/kvserver/client_replica_test.go Lines 2703 to 2705 in 40cb075
This test doesn't use span configs, and can probably just get nuked altogether. |
kv/kvserver.TestSystemZoneConfigs failed with artifacts on master @ 1e9899da8cb250a4e560b280beb2b0805ee75a78:
|
@irfansharif, what do we want to do about this? Delete the test? Do we understand why it's suddenly flaked twice in the past month? |
I have a branch that's trying to update this test to use span configs. I don't know why it's flaked -- this test is notoriously slow to run (it's disabled under stress/race), and I've not got a repro of the original failure. I'll try to push something out soon. |
I think we can get rid of the GA-blocker label since the test isn’t using span configs. The flake I think was because of initial split raceyness, span configs even without range coalescing had a few different split points compared to the system config span. And that’s what we were seeing, which this test wasn’t taught to expect. I'll try again today to get this test fixed. |
Fixes cockroachdb#98200. This test was written pre-spanconfig days, and when enabling spanconfigs by default, opted out of using it. This commit makes it use spanconfigs after giving up on reproducing/diagnosing the original flake (this test is notoriously slow -- taking 30+s given it waits for actual upreplication and replica movement, so not --stress friendly). Release note: None
Fixes cockroachdb#98200. This test was written pre-spanconfig days, and when enabling spanconfigs by default, opted out of using it. This commit makes it use spanconfigs after giving up on reproducing/diagnosing the original flake (this test is notoriously slow -- taking 30+s given it waits for actual upreplication and replica movement, so not --stress friendly). Using spanconfigs here surfaced a rare, latent bug, one this author incorrectly thought was fixed back in cockroachdb#75939. In very rare cases, right during cluster bootstrap before the span config reconciler has ever had a chance to run (i.e. system.span_configurations is empty), it's possible that the subscriber has subscribed to an empty span config state (we've only seen this happen in unit tests with 50ms scan intervals). So it's not been meaningfully "updated" in any sense of the word, but we still previously set a non-empty last-updated timestamp, something various components in KV rely on as proof that we have span configs as of some timestamp. As a result, we saw KV incorrectly merge away the liveness range into adjacent ranges, and then later split it off. We don't think we've ever seen this happen outside of tests as it instantly triggers the following fatal in the raftScheduler, which wants to prioritize the liveness range above all else: panic: priority range ID already set: old=2, new=61, first set at: Release note: None
See discussion over at #100210, this test is being rewritten and did surface a latent bug, but one that existed since 22.1. |
We have marked this test failure issue as stale because it has been |
still relevant |
Fixes cockroachdb#98200. This test was written pre-spanconfig days, and when enabling spanconfigs by default over a year ago, opted out of using it. It's a real chore to bring this old test back up to spec (cockroachdb#100210 is an earlier attempt). It has been skipped for a while after flaking (for test-only reasons that are understood, see cockroachdb#100210) and is notoriously slow taking 30+s given it waits for actual upreplication and replica movement, making it not --stress friendly. In our earlier attempt to upgrade this to use spanconfigs, we learnt two new things: - There was a latent bug, previously thought to have been fixed in cockroachdb#75939. In very rare cases, right during cluster bootstrap before the span config reconciler has ever had a chance to run (i.e. system.span_configurations is empty), it was possible that the subscriber had subscribed to an empty span config state (we've only seen this happen in unit tests with 50ms scan intervals). So it was not been meaningfully "updated" in any sense of the word, but we still previously set a non-empty last-updated timestamp, something various components in KV rely on as proof that we have span configs as of some timestamp. As a result, we saw KV incorrectly merge away the liveness range into adjacent ranges, and then later split it off. We don't think we've ever seen this happen outside of tests as it instantly triggers the following fatal in the raftScheduler, which wants to prioritize the liveness range above all else: panic: priority range ID already set: old=2, new=61, first set at: This bug continues to exist. We've filed cockroachdb#104195 to track fixing it. - Fixing the bug above (by erroring out until a span config snapshot is available) made it so that tests now needed to actively wait for a span config snapshot before relocating ranges manually or using certain kv queues. Adding that synchronization made lots of tests a whole lot slower (by 3+s each) despite reducing the closed timestamp interval, etc. These tests weren't really being harmed by the bug (== empty span config snapshot). So it's not clear that the bug fix is worth fixing. But that can be litigated in cockroachdb#104195. We don't really need this test in this current form (end-to-end spanconfig tests exist elsewhere and are more comprehensive without suffering the issues above). Release note: None
We have marked this test failure issue as stale because it has been |
104198: kvserver: kill TestSystemZoneConfigs r=irfansharif a=irfansharif Fixes #98200. This test was written pre-spanconfig days, and when enabling spanconfigs by default over a year ago, opted out of using it. It's a real chore to bring this old test back up to spec (#100210 is an earlier attempt). It has been skipped for a while after flaking (for test-only reasons that are understood, see #100210) and is notoriously slow taking 30+s given it waits for actual upreplication and replica movement, making it not --stress friendly. In our earlier attempt to upgrade this to use spanconfigs, we learnt two new things: - There was a latent bug, previously thought to have been fixed in #75939. In very rare cases, right during cluster bootstrap before the span config reconciler has ever had a chance to run (i.e. system.span_configurations is empty), it was possible that the subscriber had subscribed to an empty span config state (we've only seen this happen in unit tests with 50ms scan intervals). So it was not been meaningfully "updated" in any sense of the word, but we still previously set a non-empty last-updated timestamp, something various components in KV rely on as proof that we have span configs as of some timestamp. As a result, we saw KV incorrectly merge away the liveness range into adjacent ranges, and then later split it off. We don't think we've ever seen this happen outside of tests as it instantly triggers the following fatal in the raftScheduler, which wants to prioritize the liveness range above all else: panic: priority range ID already set: old=2, new=61, first set at: This bug continues to exist. We've filed #104195 to track fixing it. - Fixing the bug above (by erroring out until a span config snapshot is available) made it so that tests now needed to actively wait for a span config snapshot before relocating ranges manually or using certain kv queues. Adding that synchronization made lots of tests a whole lot slower (by 3+s each) despite reducing the closed timestamp interval, etc. These tests weren't really being harmed by the bug (== empty span config snapshot). So it's not clear that the bug fix is worth fixing. But that can be litigated in #104195. We don't really need this test in this current form (end-to-end spanconfig tests exist elsewhere and are more comprehensive without suffering the issues above). Release note: None Co-authored-by: irfan sharif <[email protected]>
kv/kvserver.TestSystemZoneConfigs failed with artifacts on master @ 1b162d1b274eec7b307fbbfca7294460bfdef025:
Help
See also: How To Investigate a Go Test Failure (internal)
This test on roachdash | Improve this report!
Jira issue: CRDB-25127
The text was updated successfully, but these errors were encountered: