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

kvserver: sep-raft-log: introduce struct for optional two engines #93243

Closed
tbg opened this issue Dec 8, 2022 · 3 comments
Closed

kvserver: sep-raft-log: introduce struct for optional two engines #93243

tbg opened this issue Dec 8, 2022 · 3 comments
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@tbg
Copy link
Member

tbg commented Dec 8, 2022

In a separate raft log setup, we have two engines across which a Replica's data is partitioned. We should leverage the type system to make sure these two engines can't accidentally be swapped, and introduce a struct that holds them which can serve as the main entry point for documentation of invariants and concepts. Straw man:

// Big comment here
type Storage {
  LogEngine LogEngine // if nil, not separate raft log
  StateEngine StateEngine
}

// Accessors/helpers to make it easier to support both configurations.

Epic: CRDB-220

Jira issue: CRDB-22236

@tbg tbg added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv-replication labels Dec 8, 2022
@blathers-crl
Copy link

blathers-crl bot commented Dec 8, 2022

cc @cockroachdb/replication

@tbg
Copy link
Member Author

tbg commented Dec 19, 2022

edit: oops meant to post this over on #93898.


The interesting question here is: why does the RangeTombstoneKey have to be on the state machine engine?

To see this, consider a snapshot application that also subsumes an adjacent replica r2. We need to replace it with the snapshot and also remove it, but atomically. If we have the RangeTombstoneKey on the log engine, we need extra steps to make the deletion safe (or we end up with the tombstone but the data still there, so then we risk cleaning it up even though the snapshot never applied, possibly breaking merge-related invariants, or we delete the data but then don't have a tombstone for it, making the right-hand side look like an uninitialized replica).

@tbg
Copy link
Member Author

tbg commented Feb 24, 2023

We can call this done - the Store has separated fields for the engines and accessors for them.

@tbg tbg closed this as completed Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

No branches or pull requests

1 participant