Skip to content
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

spanconfig: deflake spanconfigreconcilerccl/TestDataDriven #98082

Merged

Conversation

irfansharif
Copy link
Contributor

@irfansharif irfansharif commented Mar 6, 2023

Fixes #98038. This test set up two protection records over two schema objects at two timestamps ts=3 and ts=4.

   /Table/10{6-7}  protection_policies=[{ts: 3} {ts: 4}]
   /Table/10{7-8}  protection_policies=[{ts: 3} {ts: 4}]

When it later released those protection records:

   release record-id=3
   release record-id=4
   ----

It asserted that the span config mutations showed that we did infact get rid of the protected state:

   mutations
   ----
   delete /Table/10{6-7}
   upsert /Table/10{6-7}      range default
   delete /Table/10{7-8}
   upsert /Table/10{7-8}      range default

But since release of these protections was non-atomic, in #98038 we observed the following transition instead.

   delete /Table/10{6-7}
   upsert /Table/10{6-7}      protection_policies=[{ts: 4}]
   delete /Table/10{7-8}
   upsert /Table/10{7-8}      protection_policies=[{ts: 4}]
   delete /Table/10{6-7}
   upsert /Table/10{6-7}      range default
   delete /Table/10{7-8}
   upsert /Table/10{7-8}      range default

That is, we first got rid of the record with ts=3 and only then got rid of ts=4. We just rewrite the test to assert on the final state of the records that show no remaining protections, instead of trying to add synchronization for mutations.

Release note: None

Fixes cockroachdb#98038. This test set up two protection records over two schema
objects at two timestamps ts=3 and ts=4.

   /Table/10{6-7}  protection_policies=[{ts: 3} {ts: 4}]
   /Table/10{7-8}  protection_policies=[{ts: 3} {ts: 4}]

When it later released those protection records:

   release record-id=3
   release record-id=4
   ----

It asserted that the span config mutations showed that we did infact get
rid of the protected state:

   mutations
   ----
   delete /Table/10{6-7}
   upsert /Table/10{6-7}      range default
   delete /Table/10{7-8}
   upsert /Table/10{7-8}      range default

But since release of these protections was non-atomic, in cockroachdb#98038 we
observed the following transition instead.

   delete /Table/10{6-7}
   upsert /Table/10{6-7}      protection_policies=[{ts: 4}]
   delete /Table/10{7-8}
   upsert /Table/10{7-8}      protection_policies=[{ts: 4}]
   delete /Table/10{6-7}
   upsert /Table/10{6-7}      range default
   delete /Table/10{7-8}
   upsert /Table/10{7-8}      range default

That is, we first got rid of the record with ts=3 and only then got rid
of ts=4. We just rewrite the test to assert on the final state of the
records that show no remaining protections, instead of trying to add
synchronization for mutations.

Release note: None
@irfansharif irfansharif requested a review from a team as a code owner March 6, 2023 19:55
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@irfansharif
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 6, 2023

Build succeeded:

@craig craig bot merged commit 2853470 into cockroachdb:master Mar 6, 2023
@irfansharif irfansharif deleted the 230306.deflake-reconciler-test branch March 6, 2023 21:50
@pav-kv
Copy link
Collaborator

pav-kv commented Jun 16, 2023

blathers backport 22.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pkg/ccl/spanconfigccl/spanconfigreconcilerccl/spanconfigreconcilerccl_test: TestDataDriven failed
4 participants