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

rditer,kvserver: simplify and unify replica span handling #94201

Merged
merged 2 commits into from
Jan 17, 2023

Commits on Jan 16, 2023

  1. echotest: create directory when --rewrite passed

    Epic: none
    Release note: None
    tbg committed Jan 16, 2023
    Configuration menu
    Copy the full SHA
    afc3066 View commit details
    Browse the repository at this point in the history

Commits on Jan 17, 2023

  1. rditer,kvserver: simplify and unify replica span handling

    A Replica comprises a number of spans:
    
    - RangeID-keyed replicated data, such as the `GCThresholdKey`,
      confusingly named "local RangeID keys" since they all have a `/Local/`
    (`\x02`) prefix. They have a `r` infix.
    - RangeID-keyed unreplicated data, such as the `HardState`,
    - The system-internal key-addressable "parallel plane" (such as the
      `RangeDescriptorKey`), confusingly named "local keys" (again, `\x02`
    prefix, but `u` infix).
    - Two lock spans: one for the "local" keyspace (e.g. for an intent on
      a `RangeDescriptor`), one for the "global" keyspace (e.g. regular
      row intents), in both cases the span is local and uses a `k` infix.
    - The actual span of the range (i.e. user data).
    
    Code that needs some or all of these spans gets them from various
    methods provided by the `rditer` package. Other than being generally a
    messy affair, with a separate raft log, RangeID-based unreplicated keys
    will mostly[^1] live on the logstore engine, while RangeID-based
    replicated keys will be on the state machine engine and so the current
    approach of targeting either only the replicated *or* the combined
    (replicated and unreplicated) part of the local RangeID-based span
    no longer works. It was also never particularly obvious which spans
    one would select with that bool, and I wound up confused more than
    once.
    
    The above means we need to stop thinking about RangeID-based keys as
    belonging to "one span" - the two RangeID-keyed spans are very different
    in nature and storage.
    
    This commit introduces a unified way to select the different parts of the
    keyspace. It then goes ahead and uses this new primitive for all span
    uses external to the `rditer` package. (`rditer` itself has a few methods
    that need exotic combinations of spans, and it gets to do those more
    ad-hoc for now but this could be improved later).
    
    In particular, we can now rework `clearRangeData` and give it an
    analogoous way to explicitly select which spans to clear. Replica
    removal is a central work item in CRDB-220, since it comes up all
    over the place: snapshots (in particular in combination with replica
    subsumption), splits, replication changes, replicaGC.
    
    After this PR, we can improve splits by not mutating the raft state of
    the right-hand side replica in cases where it is known to be newer than
    the split. The old API forced us to remove the raft state as well, and
    we had to awkwardly lock the right-hand side and write the raft state
    back to make things work. We can now simply opt out of mutating that
    raft state (via `ClearUnreplicatedByRangeID=false`). This is left for
    a separate PR.
    
    This PR makes some additional improvements:
    
    - improve `rditer` tests via `echotest`, also test lock table keys
    - improved commentary, esp. around [^1]
    
    [^1]: cockroachdb#95055
    
    Release note: None
    tbg committed Jan 17, 2023
    Configuration menu
    Copy the full SHA
    40573d3 View commit details
    Browse the repository at this point in the history