logical: move TombstoneUpdater into the sqlwriter package#169462
logical: move TombstoneUpdater into the sqlwriter package#169462jeffswenson wants to merge 1 commit intocockroachdb:masterfrom
Conversation
|
✨ Submitted to Merge by @jeffswenson. It will be added to the merge queue once all branch protection rules pass and there are no merge conflicts with the target branch. See more details here. |
|
This change is |
jeffswenson
force-pushed
the
jeffswenson-move-tombstone-updater
branch
2 times, most recently
from
04ecd07 to
9dfffa6
Compare
|
Stress test failed because the sqlwriter package gained a randomized test but was failed to link in the geos package. |
aerfrei
left a comment
aerfrei
left a comment
There was a problem hiding this comment.
This faithfully moves the TombstoneUpdater. Effectively, the only thing that doesn't seem to be just a move is the return type of updateTombstone.
Looks good, excited to see the transactional LDR PR.
| return err | ||
| } | ||
| return txn.CommitInBatch(ctx, batch) | ||
| }) | ||
| }() | ||
| if err != nil { | ||
| if isLwwLoser(err) { | ||
| return batchStats{kvWriteTooOld: 1}, nil |
There was a problem hiding this comment.
Seems like we were effectively already returning a boolean here (this only took 2 values). Was there ever a vision for more from this?
There was a problem hiding this comment.
I think the idea here was that batchStats supported stats.add, so you could aggregate stats together, but it never really panned out.
This allows the tombstone updater to be reused by transactional LDR. Part of: cockroachdb#169239 Epic: CRDB-60163 Release note: None
jeffswenson
force-pushed
the
jeffswenson-move-tombstone-updater
branch
from
9dfffa6 to
1075551
Compare
3 participants
This allows the tombstone updater to be reused by transactional LDR.
Part of: #169239
Epic: CRDB-60163
Release note: None