kvserver: factor out replica loading phase #96424
Conversation
|
This change is |
pkg/kv/kvserver/replica_init.go
Outdated
| if r.mu.state.Lease.Sequence > 0 { | ||
| r.mu.minLeaseProposedTS = r.Clock().NowAsClockTimestamp() | ||
| } | ||
|
|
||
| r.assertStateRaftMuLockedReplicaMuRLocked(ctx, r.store.Engine()) |
There was a problem hiding this comment.
This line can probably go away. I didn't like the invariant break conditional to r.store.TestingKnobs().DisableEagerReplicaRemoval, but I think it should not be the case on replica creation?
There was a problem hiding this comment.
Also, assertStateRaftMuLockedReplicaMuRLocked can be implemented using loadReplicaState now, but I'm not sure what to do about this invariant exception.
pkg/kv/kvserver/replica_init.go
Outdated
| } | ||
|
|
||
| // loadReplicaState loads the state necessary to create a Replica from storage. | ||
| func loadReplicaState( |
There was a problem hiding this comment.
Can you say here what desc is in the case of an united replica?
There was a problem hiding this comment.
we could:
- change stateloader.Load to take the StartKey of the desc instead of the desc
- then load the desc from disk
- then in this method, we don't have to take a desc, but we only take a FullRangeID and an optional StartKey
- profit?
Doesn't have to happen now just wanted to put it into your head.
There was a problem hiding this comment.
Fix-up commit looks good. I think we should button this PR up and keep moving, perhaps adding a few more comments about things that are in flux in the code with a direction in which they should move. Mostly to document this in case we get pulled off the case for some reason or other.
I'm pretty interested in seeing the initialization transition cleaned up, since it's extremely brittle and verifying anything takes too much time. Let's discuss early next week.
pav-kv
force-pushed
the
load_replica
branch
4 times, most recently
from
019b301 to
09563ae
Compare
|
@tbg I reverted things a bit, so that this PR is only about factoring out the loading phase. PTAL. |
This commit factors out Replica state loading and invariant checking so that it is more consolidated rather than interleaved into Replica creation. Replica creation path was loading this state, to post-factum assert that the in-memory state matches the state in storage. Now this is a pre-requisite and input into creating the Replica, and turning it from uninitialized to initialized state. This change also eliminates the concept of "unloaded" Replica. Now the Replica is created in a valid state (either initialized or uninitialized). Release note: none Epic: none
Release note: none Epic: none
Release note: none Epic: none
Release note: none Epic: none
Release note: none Epic: none
|
bors r=tbg |
|
Build failed (retrying...): |
|
bors retry |
|
Already running a review |
|
bors retry |
|
Already running a review |
|
bors cancel |
|
Canceled. |
|
bors retry |
|
bors r=tbg |
|
Build succeeded: |
This commit factors out Replica state loading and invariant checking so that it
is more consolidated rather than interleaved into Replica creation. Replica
creation path was loading this state, to post-factum assert that the in-memory
state matches the state in storage. Now this is a pre-requisite and input into
creating the Replica, and turning it from uninitialized to initialized state.
This change also eliminates the concept of "unloaded" Replica. Now the Replica
is created in a valid state (either initialized or uninitialized).
Touches #93898