kvserver: bump the in-memory GC threshold as a pre-apply side effect #83197
Conversation
|
This change is |
aayushshah15
force-pushed
the
bumpGCThreshPreApply
branch
from
0703d62 to
be23e6f
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @aayushshah15 and @arulajmani)
pkg/kv/kvserver/replica_application_state_machine.go line 756 at r3 (raw file):
} if res.State != nil && res.State.GCThreshold != nil {
Could we add a comment here explaining why this is a pre-apply trigger? Something similar to what you have in the commit message would be great.
pkg/kv/kvserver/replica_test.go line 8652 at r3 (raw file):
testutils.RunTrueAndFalse(t, "followerRead", func(t *testing.T, followerRead bool) { testutils.RunTrueAndFalse(t, "thresholdFirst", func(t *testing.T, thresholdFirst bool) {
[tiny nit] is this now a stray line?
aayushshah15
force-pushed
the
bumpGCThreshPreApply
branch
2 times, most recently
from
ce4a73d to
c56926c
Compare
This commit changes where the in-memory GC threshold on a Replica is bumped during the application of a GC request. Previously, the in-memory GC threshold was bumped as a post-apply side effect. Additionally, GC requests do not acquire latches at the timestamp that they are garbage collecting, and thus, readers need to take additional care to ensure that results aren't served off of a partial state. Readers today rely on the invariant that the in-memory GC threshold is bumped before the actual garbage collection. This today is true because the mvccGCQueue issues GC requests in 2 phases: the first that simply bumps the in-memory GC threshold, and the second one that performs the actual garbage collection. If the in-memory GC threshold were bumped in the pre-apply phase of command application, this usage quirk wouldn't need to exist. That's what this commit does. Relates to cockroachdb#55293 Release note: None
aayushshah15
force-pushed
the
bumpGCThreshPreApply
branch
from
c56926c to
f18f31e
Compare
There was a problem hiding this comment.
Thanks for the review!
bors r+
Reviewable status:
pkg/kv/kvserver/replica_application_state_machine.go line 756 at r3 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Could we add a comment here explaining why this is a pre-apply trigger? Something similar to what you have in the commit message would be great.
Done.
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build failed: |
|
bors r+ |
|
Build succeeded: |
This commit changes where the in-memory GC threshold on a Replica is
bumped during the application of a GC request.
Previously, the in-memory GC threshold was bumped as a post-apply side
effect. Additionally, GC requests do not acquire latches at the
timestamp that they are garbage collecting, and thus, readers need to
take additional care to ensure that results aren't served off of a
partial state.
Readers today rely on the invariant that the in-memory GC threshold is
bumped before the actual garbage collection. This today is true because
the mvccGCQueue issues GC requests in 2 phases: the first that simply
bumps the in-memory GC threshold, and the second one that performs the
actual garbage collection. If the in-memory GC threshold were bumped in
the pre-apply phase of command application, this usage quirk wouldn't
need to exist. That's what this commit does.
Relates to #55293
Release note: None