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.

Sign up for GitHub

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

RFC:TiKV RawKV MVCC GC #90

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

RFC:TiKV RawKV MVCC GC #90

Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ed3ba92
RFC:TiKV RawKV MVCC GC
ystaticy Mar 22, 2022
04c39e3
RFC:TiKV RawKV MVCC GC
ystaticy Mar 22, 2022
66a5cb8
RFC:TiKV RawKV MVCC GC. add design details and flow chart
ystaticy Mar 29, 2022
92b6b54
RFC:TiKV RawKV MVCC GC. add some details.
ystaticy Mar 29, 2022
2464d0c
RFC:TiKV RawKV MVCC GC. add PD interface and pb design
ystaticy Apr 7, 2022
2186c22
feature RawKV GC,modify pd interface design
ystaticy Apr 15, 2022
15573ff
feature RawKV GC,modify interface response
ystaticy Apr 19, 2022
a86c475
feature RawKV GC,modify interface response.
ystaticy Apr 19, 2022
552fbd6
feature RawKV GC,modify interface Safepoint to SafePoint.
ystaticy Apr 19, 2022
03e416f
feature RawKV GC,modify interface name in desc
ystaticy Apr 19, 2022
dcf16a3
RFC:TiKV RawKV MVCC GC.update pd interface
ystaticy Apr 28, 2022
89194ba
RFC:TiKV RawKV MVCC GC.update etcd path
ystaticy Apr 28, 2022
3489396
RFC:TiKV RawKV MVCC GC.update compactionFilter name
ystaticy Apr 28, 2022
b211769
feature RawKV GC.modify pb info
ystaticy Apr 29, 2022
27c71e8
Merge branch 'master' into rfc_tikv_gc
ystaticy Apr 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions text/0090-tikv-gc.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# RFC: TiKV RawKV MVCC GC


## Summary
Move TiKV MVCC GC worker from TiDB into a group of independent GC worker node role and implement a new GC process in TiKV for RawKV.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the subject of this RFC is for RawKV, I think we are not discussing about "move GC worker from TiDB".

I suggest the main contents to be:

  1. Design of TiKV procedures for RawKV GC.
  2. PD meta data & interfaces.
  3. The new standalone GC worker component.
  4. Discuss whether or how TiDB can migrate to this design.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"node role" seems to be not easy to understand.
Suggest to use "component", as TiUP using this word. See tiup.io.


## Motivation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description of GC worker would be better to place in "Background" section.
I think the motivation includes:

  1. We change RawKV encoding to MVCC, so the GC is necessary.
  2. No GC for TxnKV scenario when TiDB is not deployed.
  3. The GC safe point & Txn safe point is easy to be misunderstand.

GC worker is an important component for TiKV that deletes outdated MVCC data so as to not explode the storage. But currently, the GC worker is implemented in TiDB, which makes TiKV not usable without TiDB.And current GC process is just for transaction of TiDB,it's not usable for RawKV.

## Background
According to the documentation for the current GC worker in a TiDB cluster, the GC process is as follows:

In TiDB GC worker leader:
Copy link
Contributor

@nolouch nolouch Mar 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, TiDB GC worker is not standardized use client-go. I think there are some problems that also need to be solved. details: https://docs.google.com/document/d/1jA3lK9QbYlwsvn67wGsSuusD1Dzx7ANq_vya384RBIg/edit#

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have read this document and discussed with @MyonKeminta .For implement this proposal, it need a clearer design and more development.
But due to scheduling reasons, we hope to make changes to GC as small as possible to support TiKV API V2.
So I submit this RFC.

1. Regularly calculates a new timestamp called "safe point", and push the safe point to PD.
2. Get the minimal Service safe point among all services from the response of step 2, which is GC safe point .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
2. Get the minimal Service safe point among all services from the response of step 2, which is GC safe point .
2. Get the minimal Service safe point among all services from the response of step 1, which is GC safe point .

3. Txn GC process: resolve locks and record delete ranges information.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refer to RunDistributedGCJob, where are two more steps:

  1. Save Txn safe point.
  2. Upload GC safe point to PD.

In PD leader:
1. Receive update safe point requests from TiDB or other tools (e.g. CDC, BR).
2. Calculate the minimal timestamp = min(all service safe point, now - gc_life_time).

In every TiKV nodes：
1. Get GC safe point from PD regularly.
2. Deletion will be triggered in CompactionFilter and GcTask thread;

## New GC worker architecture
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is not a new architecture. We are utilizing the same architecture but another implementation.

In a TiKV cluster without TiDB nodes , there are a few different points as follows:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In a TiKV cluster without TiDB nodes , there are a few different points as follows:
In a TiKV cluster without TiDB nodes, there are a few different points as follows:

1. We need to move GC worker into another node role.
2. For [API V2](https://github.com/tikv/rfcs/blob/master/text/0069-api-v2.md) .It need gc the earlier version in default cf. But Txn GC worker process will be triggered by WriteCompactionFilter of write cf.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
2. For [API V2](https://github.com/tikv/rfcs/blob/master/text/0069-api-v2.md) .It need gc the earlier version in default cf. But Txn GC worker process will be triggered by WriteCompactionFilter of write cf.
2. For [API V2](https://github.com/tikv/rfcs/blob/master/text/0069-api-v2.md), it need gc the earlier version in default cf. But Txn GC worker process will be triggered by WriteCompactionFilter of write cf.

3. RawKV encoded code of RawValue is different with Txn in TiDB.

So we designed a new GC architecture and process for TiKV cluster.

## Detailed design
For support TiKV cluster deploy without TiDB nodes.
1. Add a new node role instead of GC worker in TiDB nodes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to decide exactly which role before merging the RFC.

The code of new GC worker,It will be added into [tikv/migration](https://github.com/tikv/migration)
2. And for API V2, we need add new CompactionFilter which is named RawGCcompactionFilter, and add a new GCTask type implementation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a CompactionFilter for RawKV. Are we going to add a new one or just modify it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TTLCompactionFilter is just for APIV1TTL,In API V2 ,There are several differences with APIV1TTL.I think add a new CompactionFilter is better, @pingyu What do you think?

3. GC conditions in RawGCcompactionFilter is: (ts < GCSafePoint) && ( ttl-expired || deleted-mark || not the newest version ).
Copy link
Contributor

@andylokandy andylokandy Mar 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to clarify the GCSafePoint. For example (1) Do RawKV, TiDB, and TxnKV share the same GCSafePoint? (2) Who will push GCSafePoint forward?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to remove a key that ts > GCSafePoint but ttl-expired = true. @pingyu What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep such entries. Expired entries are just the same as logical deleted. Miss to capture these entries will cause downstream to return an earlier version, and being inconsistent to upstream.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pingyu So there will be this case: the CDC may sync a key that is already expired with downstream, right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(2) Who will push GCSafePoint forward?
the new node role will instead of GC Worker in TiDB, It will push GCSafePoint forward.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pingyu So there will be this case: the CDC may sync a key that is already expired with downstream, right?

Yes.

1. If the newest version is earlier than GC safe point and it's delete marked or expired ttl,those keys and earlier versions of the same userkey will be sent to a gc scheduler thread to gc asynchronous.

## Reference
https://docs.google.com/document/d/1jA3lK9QbYlwsvn67wGsSuusD1Dzx7ANq_vya384RBIg/edit#heading=h.rr3hcmc7ejb8
https://docs.pingcap.com/tidb/stable/garbage-collection-overview