[to #48] modify TiDB CDC to TiKV CDC #74

zeminzhou · 2022-03-23T03:02:51Z

What problem does this PR solve?

Issue Number: #48

Problem Description:
The current TiDB CDC should not be used for RawKV CDC.

What is changed and how does it work?

Modify TiDB CDC to synchronize RawKV data

Code changes

Has exported function/method change
Has exported variable/fields change
Has interface methods change
Has persistent data change

Check List for Tests

This PR has been tested by at least one of the following methods:

Unit test

zz-jason · 2022-03-29T05:08:41Z

cdc/cdc/capture/capture.go

-	"github.com/tikv/migration/cdc/cdc/processor/pipeline/system"
-	ssystem "github.com/tikv/migration/cdc/cdc/sorter/leveldb/system"
+
+	// ssystem "github.com/tikv/migration/cdc/cdc/sorter/leveldb/system"


why not just remove it?

The sorter module is used to sort transactions by commit ts to ensure that the transactions entered by cdc are in the same order as commit ts. Now rawkv don't it (We also need to add sorter for txnkv later).

zz-jason · 2022-03-29T05:09:20Z

cdc/cdc/capture/http_validator.go

-	if len(changefeedConfig.FilterRules) != 0 {
-		replicaConfig.Filter.Rules = changefeedConfig.FilterRules
-	}
+	/*


FilterRules are used to filter table.

We can modify the filter table to filter keys (or keyspan) to filter keys that users do not need to synchronize.

zz-jason · 2022-03-29T05:10:08Z

cdc/cdc/capture/http_validator.go

-		if err != nil {
-			return nil, cerror.ErrChangefeedUpdateRefused.GenWithStackByArgs(err.Error())
+	/*
+		if len(changefeedConfig.FilterRules) != 0 {


zz-jason · 2022-03-29T05:10:57Z

cdc/cdc/http_handler.go

-	// APIOpVarTableID is the key of table ID in HTTP API
-	APIOpVarTableID = "table-id"
+	// APIOpVarKeySpanID is the key of keyspan ID in HTTP API
+	APIOpVarKeySpanID = "keyspan-id"


what's the relationship between keyspan id and region id?

In the current implementation, keyspan and region have a one-to-one relationship. But keyspan id is not equal to region id, keyspan id only represents an interval [start, end). keyspan-id = hash(start+end).

zz-jason · 2022-03-29T05:11:31Z

cdc/cdc/kv/client.go

@@ -536,7 +536,7 @@ func (s *eventFeedSession) eventFeed(ctx context.Context, ts uint64) error {
 		}
 	})

-	tableID, tableName := util.TableIDFromCtx(ctx)
+	// tableID, tableName := util.KeySpanIDFromCtx(ctx)


There might be something wrong here, I'll check it later.

zz-jason · 2022-03-29T05:11:37Z

cdc/cdc/kv/client.go

@@ -564,7 +564,7 @@ func (s *eventFeedSession) eventFeed(ctx context.Context, ts uint64) error {
 							zap.Uint64("regionID", errInfo.singleRegionInfo.verID.GetID()),
 							zap.Uint64("ts", errInfo.singleRegionInfo.ts),
 							zap.String("changefeed", cfID), zap.Stringer("span", errInfo.span),
-							zap.Int64("tableID", tableID), zap.String("tableName", tableName),
+							// 	zap.Int64("tableID", tableID), zap.String("tableName", tableName),


zz-jason · 2022-03-29T05:12:17Z

cdc/cdc/metrics.go

@@ -19,15 +19,9 @@ import (
 	"github.com/tikv/migration/cdc/cdc/kv"
 	"github.com/tikv/migration/cdc/cdc/owner"
 	"github.com/tikv/migration/cdc/cdc/processor"
-	tablepipeline "github.com/tikv/migration/cdc/cdc/processor/pipeline"
+	keyspanpipeline "github.com/tikv/migration/cdc/cdc/processor/pipeline"


why not just use the original name pipeline?

I don't know why tablepipeline was used instead of pipeline. 🤣

zz-jason · 2022-03-29T05:13:08Z

cdc/cdc/metrics.go

-	unified.InitMetrics(registry)
-	leveldb.InitMetrics(registry)
-	redowriter.InitMetrics(registry)
+	// sorter.InitMetrics(registry)


zz-jason · 2022-03-29T05:16:03Z

cdc/cdc/owner/changefeed.go

 		feedStateManager: new(feedStateManager),
 		gcManager:        gcManager,

 		errCh:  make(chan error, defaultErrChSize),
 		cancel: func() {},

-		newDDLPuller: newDDLPuller,
-		newSink:      newDDLSink,
+		// newDDLPuller: newDDLPuller,


In TiCDC, owner is responsible for two things:

Divide changfeed into tasks and assign them to other captures.

Synchronize DML operations of upstream TiDB.

Now synchronizing DML operations don't need.

cdc/cdc/model/http_model.go

pingyu · 2022-03-29T09:37:28Z

cdc/pkg/regionspan/span.go

@@ -37,6 +38,15 @@ func (s Span) String() string {
 	return fmt.Sprintf("[%s, %s)", hex.EncodeToString(s.Start), hex.EncodeToString(s.End))
 }

+func (s Span) ID() uint64 {


There is a very small possibility that two different spans generating a same hash. Maybe we should add a seed in Span to avoid it.

Yes, it is indeed possible. But I don't think adding a seed is effective, because this is a probabilistic event.

a simple solution, https://github.com/zeminzhou/migration/blob/tikv-cdc/cdc/cdc/owner/scheduler_v1.go#L446-L453.

There is still conflict between "currentKeySpans" and the keyspans acquired before.

How about using region id as keyspan id?https://github.com/zeminzhou/migration/blob/tikv-cdc/cdc/cdc/owner/scheduler_v1.go#L444.

If we do it this way, keyspan will lose its meaning.

No. The range of a region will change after split or merge.

How about add a seed to KeySpan to avoid conflict ?

We maintain all key spans in owner. Every time we get a new key span, calculate the hash with seed starting from 0. If the hash conflicts with an existing key span, seed += 1, and calculate the hash again. Until we find a hash without conflict.
Note that the seed should be transfer to Capture & Processor, along with key span's start & end.

pingyu · 2022-03-29T10:14:24Z

cdc/cdc/owner/owner.go

@@ -58,7 +58,7 @@ type ownerJob struct {
 	// for ManualSchedule only
 	targetCaptureID model.CaptureID
 	// for ManualSchedule only
-	tableID model.TableID


(Not related to this line)
GC related procedure (Owner.updateGCSafepoint) need to be reviewed. At least the CDCServiceSaftPointID must be changed. Refer to rfc.

cdc/cdc/owner/owner_test.go

cdc/cdc/owner/metrics.go

cdc/cdc/owner/scheduler_v1.go

pingyu · 2022-03-29T11:58:55Z

cdc/cdc/owner/scheduler_v1.go


 	s.cleanUpFinishedOperations()
-	pendingJob, err := s.syncTablesWithCurrentTables()
+	pendingJob, err := s.syncKeySpansWithCurrentKeySpans()


Keyspans to add & keyspans to remove, must be overlapped. So to be correct, I think we must finish remove jobs first, and then dispatch the add jobs.

The "remove then add" actions would be expensive, and make the replication be jitter. So I think it would be better to limit the number of keyspans to be balanced in every period.

If we limit the number of keyspans to be balanced, the keyspans should be "related", e.g, the splitted regions and the original region, the merged region and the orginal two regions.

cdc/cdc/owner/scheduler_v1.go

pingyu · 2022-04-14T16:28:52Z

cdc/cdc/entry/metrics.go

@@ -20,22 +20,22 @@ import (
 var (


These metrics is for mounter. Can be removed.

cdc/cmd/kafka-consumer/main.go

cdc/cdc/sink/buffer_sink.go

cdc/cdc/sink/common/flow_control.go

cdc/pkg/cmd/cli/cli_changefeed.go

Signed-off-by: zeminzhou <[email protected]>

Issue Number: #57 Signed-off-by: pingyu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

Co-authored-by: Ping Yu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

Signed-off-by: zeminzhou <[email protected]>

Co-authored-by: Ping Yu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

Signed-off-by: zeminzhou <[email protected]>

* [to #67] setup backup/restore integration test for rawkv Signed-off-by: Jian Zhang <[email protected]> * add github action Signed-off-by: Jian Zhang <[email protected]> Signed-off-by: zeminzhou <[email protected]>

* [to #67] remove some unused code Signed-off-by: Jian Zhang <[email protected]> * remove web Signed-off-by: Jian Zhang <[email protected]> Co-authored-by: Ping Yu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

Signed-off-by: Jian Zhang <[email protected]> Signed-off-by: zeminzhou <[email protected]>

Signed-off-by: zeminzhou <[email protected]>

* br: cherry pick tidb#32612 to fix rawkv backup failure Issue Number: #67 Signed-off-by: pingyu <[email protected]> * revert unnecessary port change Signed-off-by: pingyu <[email protected]> * migrate test logics from run.sh to run.py Signed-off-by: pingyu <[email protected]> * temporarily disable old versions < 6.0 Signed-off-by: pingyu <[email protected]> * separate CI of 5.X versions Signed-off-by: pingyu <[email protected]> * bugfix Signed-off-by: pingyu <[email protected]> * bugfix Signed-off-by: pingyu <[email protected]> * address comments Signed-off-by: pingyu <[email protected]> * tiny fix Signed-off-by: pingyu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

Signed-off-by: zeminzhou <[email protected]>

pingyu

LGTM~

Left over issues:

zeminzhou requested a review from pingyu March 23, 2022 03:02

github-actions bot added component/cdc component/br component/bulk-load component/sst-data-source and removed component/br component/sst-data-source component/bulk-load labels Mar 23, 2022

zz-jason reviewed Mar 29, 2022

View reviewed changes

pingyu reviewed Mar 29, 2022

View reviewed changes

pingyu reviewed Apr 14, 2022

View reviewed changes

cdc/cdc/entry/metrics.go

@@ -20,22 +20,22 @@ import (

var (

Copy link

Collaborator

pingyu Apr 14, 2022

Choose a reason for hiding this comment

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

These metrics is for mounter. Can be removed.

pingyu reviewed Apr 18, 2022

View reviewed changes

cdc/cmd/kafka-consumer/main.go Show resolved Hide resolved

cdc/cdc/sink/buffer_sink.go Show resolved Hide resolved

cdc/cdc/sink/common/flow_control.go Outdated Show resolved Hide resolved

cdc/pkg/cmd/cli/cli_changefeed.go Outdated Show resolved Hide resolved

zeminzhou and others added 16 commits April 18, 2022 15:11

modify TiCDC to TiKV CDC

a330434

Signed-off-by: zeminzhou <[email protected]>

br: fix CI timeout (#71)

d537173

Issue Number: #57 Signed-off-by: pingyu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

[to #67] remove unused function and dead code (#69)

6acd595

Co-authored-by: Ping Yu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

fix lint

8eea41e

Signed-off-by: zeminzhou <[email protected]>

fix ut

2956d83

Signed-off-by: zeminzhou <[email protected]>

fix ut

92a5940

Signed-off-by: zeminzhou <[email protected]>

[to #519] move br component from tikv java client to this repo (#73)

fe8cac1

Signed-off-by: zeminzhou <[email protected]>

[to #67] remove unused code related to restore (#76)

be501e8

Signed-off-by: zeminzhou <[email protected]>

fix ut

4cf9276

Signed-off-by: zeminzhou <[email protected]>

[to #67] remove unused function and dead code (#69)

c829e71

Co-authored-by: Ping Yu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

[to #67] remove unused code related to restore (#76)

62c7f00

Signed-off-by: zeminzhou <[email protected]>

remove ForceReplicate, IgnoreIneligibleTable, IgnoreTxnStartTs, Mounter

6bd0a5c

Signed-off-by: zeminzhou <[email protected]>

fix the same keyspan id

e0a7e8b

Signed-off-by: zeminzhou <[email protected]>

fix the same keyspan id

bd77c3a

Signed-off-by: zeminzhou <[email protected]>

namespace ticdc -> tikv_cdc

d8eec09

Signed-off-by: zeminzhou <[email protected]>

change func name to updateCurrentKeySpansImpl

a61dd5a

Signed-off-by: zeminzhou <[email protected]>

zeminzhou and others added 16 commits April 18, 2022 15:15

fix hard code

f60b0fe

Signed-off-by: zeminzhou <[email protected]>

fix error name

4380a2f

Signed-off-by: zeminzhou <[email protected]>

fix ut

7f5419a

Signed-off-by: zeminzhou <[email protected]>

add deleted ut TestFixChangefeedSinkProtocol

40c9ecb

Signed-off-by: zeminzhou <[email protected]>

[to #66] update kvproto to support api v2 (#79)

687fea1

Signed-off-by: zeminzhou <[email protected]>

[to #67] remove unused code related to parser (#78)

3363a0b

Signed-off-by: zeminzhou <[email protected]>

[to #67] clean Makefile (#80)

c5da5e5

Signed-off-by: zeminzhou <[email protected]>

fix keyspan id

a878f96

Signed-off-by: zeminzhou <[email protected]>

[to #67] setup backup/restore integration test for rawkv (#82)

fbdefb7

* [to #67] setup backup/restore integration test for rawkv Signed-off-by: Jian Zhang <[email protected]> * add github action Signed-off-by: Jian Zhang <[email protected]> Signed-off-by: zeminzhou <[email protected]>

[to#67] remove some unused code (#81)

be8b5dd

* [to #67] remove some unused code Signed-off-by: Jian Zhang <[email protected]> * remove web Signed-off-by: Jian Zhang <[email protected]> Co-authored-by: Ping Yu <[email protected]> Signed-off-by: zeminzhou <[email protected]>

remove unnecessary tests (#83)

78ad01b

Signed-off-by: Jian Zhang <[email protected]> Signed-off-by: zeminzhou <[email protected]>

[to #67] support setting api version in br requests (#75)

d890254

Signed-off-by: zeminzhou <[email protected]>

[to #67] remove unnecessary codes (#84)

36f9ee8

Signed-off-by: zeminzhou <[email protected]>

fix comment

fc51e96

Signed-off-by: zeminzhou <[email protected]>

fix comment

7d0ee78

Signed-off-by: zeminzhou <[email protected]>

github-actions bot added component/br component/bulk-load component/sst-data-source labels Apr 18, 2022

Merge branch 'main' into tikv-cdc

eede5d2

github-actions bot removed component/bulk-load component/sst-data-source component/br labels Apr 18, 2022

pingyu approved these changes Apr 18, 2022

View reviewed changes

pingyu merged commit 3b82376 into tikv:main Apr 18, 2022

pingyu mentioned this pull request Apr 18, 2022

[to #48] Create actions for CDC #86

Merged

zeminzhou mentioned this pull request Apr 20, 2022

[to #48] try to fix scheduler problem #90

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[to #48] modify TiDB CDC to TiKV CDC #74

[to #48] modify TiDB CDC to TiKV CDC #74

zeminzhou commented Mar 23, 2022

zz-jason Mar 29, 2022

zeminzhou Mar 29, 2022 •

edited

Loading

zz-jason Mar 29, 2022

zeminzhou Mar 29, 2022

zeminzhou Mar 29, 2022

zz-jason Mar 29, 2022

zz-jason Mar 29, 2022

zeminzhou Mar 29, 2022

zz-jason Mar 29, 2022

zeminzhou Mar 29, 2022

zz-jason Mar 29, 2022

zz-jason Mar 29, 2022

zeminzhou Mar 29, 2022

zz-jason Mar 29, 2022

zz-jason Mar 29, 2022

zeminzhou Mar 29, 2022 •

edited

Loading

pingyu Mar 29, 2022

zeminzhou Mar 30, 2022

zeminzhou Mar 30, 2022

pingyu Mar 30, 2022

zeminzhou Apr 1, 2022

pingyu Apr 14, 2022

pingyu Mar 29, 2022

pingyu Mar 29, 2022

pingyu Apr 14, 2022

pingyu left a comment

[to #48] modify TiDB CDC to TiKV CDC #74

[to #48] modify TiDB CDC to TiKV CDC #74

Conversation

zeminzhou commented Mar 23, 2022

What problem does this PR solve?

What is changed and how does it work?

Code changes

Check List for Tests

Choose a reason for hiding this comment

zeminzhou Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeminzhou Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pingyu left a comment

Choose a reason for hiding this comment

zeminzhou Mar 29, 2022 •

edited

Loading

zeminzhou Mar 29, 2022 •

edited

Loading