Skip to content

RFC:TiKV RawKV MVCC GC#90

Open
ystaticy wants to merge 15 commits into
tikv:masterfrom
ystaticy:rfc_tikv_gc
Open

RFC:TiKV RawKV MVCC GC#90
ystaticy wants to merge 15 commits into
tikv:masterfrom
ystaticy:rfc_tikv_gc

Conversation

@ystaticy

Copy link
Copy Markdown

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.

Motivation

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.

Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Comment thread text/0090-tikv-gc.md Outdated

## 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
Copy Markdown
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.

Comment thread text/0090-tikv-gc.md Outdated
For support TiKV cluster deploy without TiDB nodes.
1. Add a new node role instead of GC worker in TiDB nodes.
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
Copy Markdown
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
Copy Markdown
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?

Comment thread text/0090-tikv-gc.md Outdated
1. Add a new node role instead of GC worker in TiDB nodes.
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.
3. GC conditions in RawGCcompactionFilter is: (ts < GCSafePoint) && ( ttl-expired || deleted-mark || not the newest version ).

@andylokandy andylokandy Mar 22, 2022

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@BusyJay

BusyJay commented Mar 22, 2022

Copy link
Copy Markdown
Member

The RFC is quite simple yet. For example, how is the new node designed? How it triggers GC? How does GC work in details? What's the triggering frequency?

Comment thread text/0090-tikv-gc.md
## 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:

@nolouch nolouch Mar 23, 2022

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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.

@ystaticy

Copy link
Copy Markdown
Author

@BusyJay OK,I'll add more details for this RFC

Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Comment thread text/0090-tikv-gc.md Outdated


## 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
Copy Markdown
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
Copy Markdown
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.

Comment thread text/0090-tikv-gc.md
## 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.

## Motivation

Copy link
Copy Markdown
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.

Comment thread text/0090-tikv-gc.md Outdated
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.

## Motivation
1.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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
1.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.
1. 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.

Comment thread text/0090-tikv-gc.md Outdated

In TiDB GC worker leader:
1. Regularly calculates a new timestamp called "GC safe point"(The default interval is 10min), 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
Copy Markdown
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 .

Comment thread text/0090-tikv-gc.md
1. Regularly calculates a new timestamp called "GC safe point"(The default interval is 10min), 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 .
3. Txn GC process: resolve locks and record delete ranges information.

Copy link
Copy Markdown
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.

Comment thread text/0090-tikv-gc.md Outdated
For support RawKV GC in TiKV cluster deploy without TiDB nodes.
1. Add a new node role instead of GC worker in TiDB nodes.
- Why we choose to create a new node role:
- IF we add GC Worker in PD: It will cause the problem of client-go circular dependency.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the main reason is that PD is designed for scheduling & meta storage. Acting as GC worker will bring inappropriate feature and lower availability of PD.

Comment thread text/0090-tikv-gc.md Outdated
- Due to TiDB, TxnKV and RawKV are allowed to coexist. Because the data of the three scenarios are independent, Because the data of the three scenarios are independent, separate safepoints are used in the GC, which helps to reduce the interference between businesses and speed up the GC.
- If multi tenancy is supported in the future, 'service group' can also support it.
- Need to design new interfaces for update service safepoint with 'service group'.
- Add UpdateServiceGCSafepointByServiceGroup and getGCSafepointByServiceGroup to standardize the API.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please also specify the arguments and returns, as well as the main logics of the interface.

The naming is a little too long. "UpdateServiceGCSafepointByGroup" may be better.

How to allocate $service_group_id ?

What's the path of GC safepoint ?

Will there be new interface & ETCD path for Txn safepoint ? TxnKV will need a Txn safepoint.

Comment thread text/0090-tikv-gc.md Outdated
1. Get GC safe point from PD regularly.
2. Deletion will be triggered in CompactionFilter and GcTask thread;

## New GC worker architecture

Copy link
Copy Markdown
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.

Comment thread text/0090-tikv-gc.md Outdated
## New GC worker architecture
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
Copy Markdown
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.

ystaticy added 2 commits April 7, 2022 08:57
Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Comment thread text/0090-tikv-gc.md Outdated
2. used to get GC safepoint for TiKV:
1. interface:
```shell
func (s *GrpcServer) GetAllServiceGroupGcSafePoint(ctx context.Context, request *pdpb.GetServiceGroupServiceGcSafeRequest) (*pdpb.UpdateServiceGCSafePointResponse, error)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should Gc here be capitalized to stay consistent with other interfaces?

Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Signed-off-by: ystaticy <y_static_y@sina.com>
Comment thread text/0090-tikv-gc.md
Move TiKV MVCC GC worker from TiDB into a group of independent GC worker component and implement a new GC process in TiKV for RawKV.

## Motivation
1. 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
1. 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.
1. 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.

Comment thread text/0090-tikv-gc.md

In TiDB GC worker leader:
1. Regularly calculates a new timestamp called "GC safe point"(The default interval is 10min), and push the safe point to PD.
2. Get the minimal Service safe point among all services from the response of step 1, which is GC safe point .

Copy link
Copy Markdown
Member

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 1, 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 .

Comment thread text/0090-tikv-gc.md
2. Deletion will be triggered in CompactionFilter and GcTask thread;

## New GC worker implementation
In a TiKV cluster without TiDB nodes , there are a few different points as follows:

Copy link
Copy Markdown
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:

Comment thread text/0090-tikv-gc.md
## New GC worker implementation
In a TiKV cluster without TiDB nodes , there are a few different points as follows:
1. We need to move GC worker into another component.
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
Copy Markdown
Member

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 needs gc the earlier version in default cf. But Txn GC worker process will be triggered by WriteCompactionFilter of write cf.

Comment thread text/0090-tikv-gc.md
RequestHeader header = 1;
bytes service_group_id = 2;
bytes service_id = 3;
int64 TTL = 4;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

TTL doesn't follow field name standard. If ttl generates strange field accessor, we can change it to xxx_ttl or maybe time_to_live. They both look better and consistent.

Comment thread text/0090-tikv-gc.md
2. Changes on PD:
1. A new concept is 'service group':
- Due to TiDB, TxnKV and RawKV are allowed to coexist. Because the data of the three scenarios are independent, Because the data of the three scenarios are independent, separate safepoints are used in the GC, which helps to reduce the interference between businesses and speed up the GC.
- If multi tenancy is supported in the future, 'service group' can also support it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where do we put start and end key for a service group?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants