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

kvnemesis: use execution timestamp from ambiguous errors #88722

Closed
wants to merge 3 commits into from

Conversation

tbg
Copy link
Member

@tbg tbg commented Sep 26, 2022

Now whenever data actually made it to KV (ambiguously or not) we know
the timestamp at which this has happened.

We can thus remove much of the special-casing around deletions. In
particular, we no longer need to consider half-materialized writes,
i.e. the case Materialized=true && Timestamp.IsEmpty(). In turn,
we get to remove the Materialized field altogether, as it now
always equals Timestamp.IsSet(). This then removes all special
casing around deletions. They are still keyed by key and timestamp
as opposed to by value (since they have no value) but other than
that they get treated the same.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg tbg force-pushed the ambiguous-candidate-timestamps branch from 2b9161b to c875758 Compare September 26, 2022 15:28
tbg added a commit to tbg/cockroach that referenced this pull request Sep 26, 2022
This is possible now, thanks to cockroachdb#88722.

Closes cockroachdb#69642.

Release note: None
tbg added 2 commits September 27, 2022 11:05
Now whenever data actually made it to KV (ambiguously or not) we know
the timestamp at which this has happened.

We can thus remove much of the special-casing around deletions. In
particular, we no longer need to consider half-materialized writes,
i.e. the case `Materialized=true && Timestamp.IsEmpty()`. In turn,
we get to remove the `Materialized` field altogether, as it now
always equals `Timestamp.IsSet()`. This then removes all special
casing around deletions. They are still keyed by key and timestamp
as opposed to by value (since they have no value) but other than
that they get treated the same.

Release note: None
@tbg tbg force-pushed the ambiguous-candidate-timestamps branch from c875758 to ee8de81 Compare September 27, 2022 10:58
@tbg
Copy link
Member Author

tbg commented Sep 27, 2022

Big rework coming in a day or two.

@tbg
Copy link
Member Author

tbg commented Oct 6, 2022

"a day or two"

Well here it is #89477

@tbg tbg closed this Oct 6, 2022
@tbg tbg deleted the ambiguous-candidate-timestamps branch October 6, 2022 20:09
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.

2 participants