Skip to content

keys: standardize replicated and unreplicated key names #7357

@tbg

Description

@tbg

From discussion on #7310:

@tschottdorf wrote:

You're right, the renames are a little too drive-by. I've removed them since they don't need to be in this PR. In principle though, I think the new names are better - both keys are counted as replicated keys because they are replicated - they are present in the snapshot, and they are mutated only through Raft. As such, they are part of the replica state (as in, the replica state machine state), and there's no difference between the applied index, and, say, the leader lease or frozen status. Our naming for these keys is RangeSomethingSomethingKey, so I wanted to homogenize in that way. Maybe I don't understand the naming here right?

BTW, what's up with RaftTombstoneKey above? Does that need to/should it be replicated?

@bdarnell wrote:

I don't feel strongly about the names. Our current naming is kind of a mess, but even with this change, starting with Range is not a reliable indicator for replicated range-id-local keys (AbortCacheKey is another one in this category, while RangeLast{ReplicaGC,Verification}TimestampKey start with Range but are not replicated. If we want to standardize on Range for replicated local keys (or RangeID to distinguish from the range descriptor, range tree, and transaction keys) and Replica for unreplicated ones I'd be OK with that.

I think the replicated status of RaftTombstoneKey is a bug, that manages to not matter in practice because it appears only on stores that are not currently a member of the group.

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-cleanupTech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions