gc: use clear range operations to remove multiple garbage keys across keys and versions by aliher1911 · Pull Request #90830 · cockroachdb/cockroach

aliher1911 · 2022-10-28T12:40:29Z

Summary:

GC Will track consecutive keys and issue GCClearRange requests to remove multiple keys at once if it can find more than configured number of keys. This covers multiple versions of the same key as well as multiple consecutive keys.

On the GC side it will count consecutive keys while building "points batches" e.g. batches containing MVCC keys to delete. Those batches are capped to fit into resulting raft entries once deletion is executed. GC could accumulate more than a single batch in memory in that case. Once necessary number of consecutive keys is found, GCClearRange is issued and pending batches are discarded. If non gc-able data is found, pending batches are GC'd and clear range tracking is restarted.
To protect GC from consuming too much memory for example if it collects a range with very large keys, it uses a configrable memory limit on how many key bytes could be held in pending batches. If this limit is reached before GC reaches minimum desired number of keys, oldest batch is GC'd and starting point for clear range is adjusted.

On the evaluation side, GC requests for ranges will obtain write latches removed range to prevent other requests writing in the middle of the range. However, if only a single key is touched i.e. multiple versions are deleted, then no latches are obtained as it is done when deleting individual versions as all the removed data is below the GC threshold.

GC exposes a metric queue.gc.info.numclearconsecutivekeys that shows how many times GCClearRange was used.
GC adds new system setting kv.gc.clear_range_min_keys that sets minimum number of keys eligible for GCClearRange.

batcheval: handle ClearSubRange request fields in GC requests

Handle ClearSubRange GC request fields to use GCClearRange storage
functions for ranges of keys that contain no visible data instead
of removing each key individually.
This PR only enables processing of request field, while actual GC
and storage functionality is added in previous PRs.

Release note: None

gc: add version gate to GC clear range requests

ClearRange fields of GCRequests are ignored by earlier versions,
this feature needs to be version gated to let GC work when talking
to a previous version if GC is run from a different node.

Release note: None

gc: add metrics for clear range requests in GC

This commit adds two metrics for clear range functionality in GC:

queue.gc.info.numclearconsecutivekeys is incremented every time GC sends
a ClearRange request for a bunch of consecutive keys to a replica

Release note (ops change): Metric queue.gc.info.numclearconsecutivekeys
added. It exposes number of clear range requests to remove consecutive
keys issued by mvcc garbage collection.

gc: GC use delete_range_keys for consecutive ranges of keys

When deleting large chunks of keys usually produced by range tombstones
GC will use point deletion for all versions found within replica key
range.
That kind of operations would generate unnecessary load on the storage
because each individual version must be deleted independently.
This patch adds an ability for GC to find ranges of consecutive keys
which are not interleaved with any newer data and create
delete_range_key GC requests to remove them.

Release note: None

rditer: helper to visit multiple replica key spans

IterateMVCCReplicaKeySpans helper to complement IterateplicaKeySpans
Existing tests moved from ReplicaMVCCDataIterator to
IterateMVCCReplicaKeySpans as former is being sunsetted and only a
single call site exists.

Release note: None

storage: add gc operation that uses clear range

Deleting multiple versions of same key or continuous ranges of
point keys with pebble tombstones could be inefficient when range
has lots of garbage.
This pr adds a storage gc operation that uses pebble range tombstones
to remove data.

Release note: None

gc: add delete range parameters to gc request

This commit adds clear_sub_range_key parameters to GC request.
clear_sub_range_keys is used by GC queue to remove chunks of
consecutive keys where no new data was written above the GC
threshold and storage can optimize deletions with range
tombstones.
To support new types of keys, GCer interface is also updated
to pass provided keys down to request.

Release note: None

Fixes: #84560, #61455

cockroach-teamcity · 2022-10-28T12:40:43Z

This change is

dhartunian

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @aliher1911 and @erikgrinaker)

-- commits line 57 at r7:
can you include a release note for the new metric?

erikgrinaker

Flushing some comments after an initial high-level pass. I need to do a more thorough pass later, in particular of the GC batching code.

pkg/clusterversion/cockroach_versions.go

pkg/roachpb/api.proto

pkg/storage/mvcc.go

pkg/kv/kvserver/gc/gc.go

pkg/kv/kvserver/rditer/replica_data_iter_test.go

pkg/kv/kvserver/gc/gc_random_test.go

pkg/kv/kvserver/gc/gc.go

pkg/kv/kvserver/batcheval/cmd_gc.go

pkg/roachpb/api.proto

erikgrinaker

I still need to do another pass over the batch logic, but I've re-reviewed the rest.

pkg/kv/kvserver/rditer/replica_data_iter_test.go

pkg/storage/mvcc.go

erikgrinaker · 2022-12-01T15:46:52Z

pkg/storage/testdata/mvcc_histories/gc_clear_full_range

Let's add a couple of variants that check a live value and a point tombstone that are exactly on the GC threshold too (or do we have them already?).

Not much sense for the full range deletion. I add one more test but overall we bail if there's anything not masked by range tombstone even if technically it should be collectable. E.g. point tombstone followed by range tombstone is not enough for fast path to succeed.

But I'll check if we can add more for non optimized case.

The motivation is that I want to make sure we have test coverage for this branch, and the corresponding branch in the full-range deletion:

https://github.com/cockroachdb/cockroach/pull/90830/files#diff-2a418618ad51e1cdef4b44240ef9ee4789e9f97f2d6d9734b67ea7fc94d5d9afR5452-R5457

I don't believe we currently have any test cases that hit it?

Actually I think this condition is not needed at all. If key is garbage then it must be covered by newer value (its timestamp must be below coveredBy). And coveredBy timestamp is guaranteed to be at or below gcThreshold so we must be good.

erikgrinaker · 2022-12-01T15:48:51Z

pkg/kv/kvserver/gc/gc.go

Yeah, I think we discussed not GCing range keys if point key GC failed.

erikgrinaker · 2022-12-02T14:03:05Z

pkg/kv/kvserver/gc/gc.go

Can this be equal to the last key? Doesn't it have to be greater than it in order to cover it?

Also, this should be "no keys" rather than "no garbage keys", right? Because any live keys would be removed.

Yeah, that is correct. It could be last when we start iterating, but we will always tighten it to the batch[0][0].Next() before sending.

erikgrinaker

The GC batcher LGTM, feel free to merge once comments have been addressed.

erikgrinaker · 2022-12-02T14:19:36Z

PS: this can close out #61455 too.

This commit adds timestamp field to clear_range_key parameters of GC request. New timestamp parameter is used by GC queue to remove chunks of consecutive keys where no new data was written above the GC threshold and storage can optimize deletions with range tombstones. Release note: None

Deleting multiple versions of same key or continuous ranges of point keys with pebble tombstones could be inefficient when range has lots of garbage. This pr adds a storage gc operation that uses pebble range tombstones to remove data. Release note: None

IterateMVCCReplicaKeySpans helper to complement IterateplicaKeySpans Existing tests moved from ReplicaMVCCDataIterator to IterateMVCCReplicaKeySpans as former is being sunsetted and only a single call site exists. Release note: None

When deleting large chunks of keys usually produced by range tombstones GC will use point deletion for all versions found within replica key range. That kind of operations would generate unnecessary load on the storage because each individual version must be deleted independently. This patch adds an ability for GC to find ranges of consecutive keys which are not interleaved with any newer data and create clear_range_key GC requests to remove them. Release note: None

This commit adds metrics to GC ClearRange requests in GC. Metrics are shared with with existing GC ClearRangeKey full range deletions since they are peforming similar operation and exposed as queue.gc.info.clearrangesuccess/queue.gc.info.clearrangefailed. Release note (ops change): Metrics queue.gc.info.clearrangesuccess queue.gc.info.clearrangefailed updated to include statistics about GC operations that perform ClearRange on parts of range keyspace. Previously those metrics only included requests to remove range data completely when performing a schema change.

ClearRange fields of GCRequests are ignored by earlier versions, this feature needs to be version gated to let GC work when talking to a previous version if GC is run from a different node. Release note: None

Handle ClearRange GC request fields to use GCClearRange storage functions for ranges of keys that contain no visible data instead of removing each key individually. This PR only enables processing of request field, while actual GC and storage functionality is added in previous PRs. Release note: None

aliher1911 · 2022-12-19T16:45:43Z

bors r=erikgrinaker

craig · 2022-12-19T18:12:36Z

Build failed (retrying...):

Bazel Essential CI (Cockroach)

craig · 2022-12-19T19:51:39Z

Build succeeded:

Bazel Essential CI (Cockroach)

aliher1911 force-pushed the clear_range_gc_7 branch 2 times, most recently from cac19b4 to 42e5b13 Compare October 28, 2022 23:18

aliher1911 self-assigned this Oct 28, 2022

aliher1911 force-pushed the clear_range_gc_7 branch 3 times, most recently from 549ddf1 to 98b1a41 Compare October 31, 2022 10:46

aliher1911 changed the title ~~gc: add delete range parameters to gc request~~ gc: use clear range operations to remove multiple garbage keys across keys and versions Oct 31, 2022

aliher1911 force-pushed the clear_range_gc_7 branch 4 times, most recently from 92bc9b3 to ed74dcc Compare October 31, 2022 15:33

aliher1911 marked this pull request as ready for review October 31, 2022 17:17

aliher1911 requested a review from a team October 31, 2022 17:17

aliher1911 requested review from a team as code owners October 31, 2022 17:17

aliher1911 requested a review from erikgrinaker October 31, 2022 17:18

dhartunian reviewed Nov 1, 2022

View reviewed changes

erikgrinaker reviewed Nov 9, 2022

View reviewed changes

aliher1911 force-pushed the clear_range_gc_7 branch from ed74dcc to 7df3f66 Compare November 16, 2022 11:53

erikgrinaker reviewed Nov 21, 2022

View reviewed changes

aliher1911 force-pushed the clear_range_gc_7 branch 9 times, most recently from ffb73af to f80f540 Compare November 28, 2022 14:36

aliher1911 requested a review from erikgrinaker November 28, 2022 22:16

erikgrinaker reviewed Dec 1, 2022

View reviewed changes

pkg/kv/kvserver/batcheval/cmd_gc.go Outdated Show resolved Hide resolved

pkg/kv/kvserver/batcheval/cmd_gc.go Outdated Show resolved Hide resolved

pkg/kv/kvserver/batcheval/cmd_gc.go Outdated Show resolved Hide resolved

pkg/roachpb/api.proto Outdated Show resolved Hide resolved

erikgrinaker reviewed Dec 1, 2022

View reviewed changes

erikgrinaker reviewed Dec 2, 2022

View reviewed changes

erikgrinaker approved these changes Dec 2, 2022

View reviewed changes

erikgrinaker mentioned this pull request Dec 2, 2022

roachpb,gc,mvcc: add UseClearRange option to GCRequest_GCKey #51230

Closed

aliher1911 force-pushed the clear_range_gc_7 branch 3 times, most recently from f226f81 to 1da6d73 Compare December 7, 2022 18:01

aliher1911 added 3 commits December 19, 2022 10:05

rditer: helper to visit multiple replica key spans

d139cac

IterateMVCCReplicaKeySpans helper to complement IterateplicaKeySpans Existing tests moved from ReplicaMVCCDataIterator to IterateMVCCReplicaKeySpans as former is being sunsetted and only a single call site exists. Release note: None

aliher1911 force-pushed the clear_range_gc_7 branch from 1da6d73 to c3e941f Compare December 19, 2022 12:44

aliher1911 requested a review from a team December 19, 2022 12:44

aliher1911 added 4 commits December 19, 2022 16:44

gc: add version gate to GC clear range requests

e7da89d

ClearRange fields of GCRequests are ignored by earlier versions, this feature needs to be version gated to let GC work when talking to a previous version if GC is run from a different node. Release note: None

aliher1911 force-pushed the clear_range_gc_7 branch from c3e941f to 9efbf4f Compare December 19, 2022 16:45

craig bot merged commit 91fbbad into cockroachdb:master Dec 19, 2022

cockroach-teamcity mentioned this pull request Dec 20, 2022

PR #90830 - gc: add metrics for clear range requests in GC cockroachdb/docs#15864

Open

erikgrinaker mentioned this pull request Dec 20, 2022

kvserver: add UseClearRange option to GCRequest_GCKey #61455

Closed

This was referenced Mar 7, 2023

gc: inject bugs in GC to verify randomized test coverage. #98156

Closed

gc: benchmark effects of clear range on write heavy workloads #98157

Closed

pav-kv mentioned this pull request Jan 30, 2026

kvserver: inconsistency due to race in MVCC GC #162085

Closed

Conversation

aliher1911 commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Oct 28, 2022

Uh oh!

dhartunian left a comment

Choose a reason for hiding this comment

Uh oh!

erikgrinaker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erikgrinaker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erikgrinaker Dec 1, 2022

Choose a reason for hiding this comment

Uh oh!

aliher1911 Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erikgrinaker Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aliher1911 Dec 19, 2022

Choose a reason for hiding this comment

Uh oh!

erikgrinaker Dec 1, 2022

Choose a reason for hiding this comment

Uh oh!

erikgrinaker Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

aliher1911 Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

erikgrinaker left a comment

Choose a reason for hiding this comment

Uh oh!

erikgrinaker commented Dec 2, 2022

Uh oh!

aliher1911 commented Dec 19, 2022

Uh oh!

craig bot commented Dec 19, 2022

Uh oh!

craig bot commented Dec 19, 2022

aliher1911 commented Oct 28, 2022 •

edited

Loading

aliher1911 Dec 7, 2022 •

edited

Loading

erikgrinaker Dec 8, 2022 •

edited

Loading