sorter/leveldb(ticdc): replace cleaner with delete range #4632

overvenus · 2022-02-19T09:31:40Z

What problem does this PR solve?

Add DeleteRange API to DB
Update DB dashboard in grafana
Adjust DB block cahce by sorter.max-memory-percentage

Issue Number: ref #4631

Check List

Tests

Unit test
Integration test

Release note

None

* Add DeleteRange API to DB * Update DB dashboard in grafana * Adjust DB block cahce by sorter.max-memory-percentage Signed-off-by: Neil Shen <[email protected]>

ti-chi-bot · 2022-02-19T09:31:41Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

sdojjy
zhaoxinyu

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

codecov-commenter · 2022-02-19T09:47:24Z

Codecov Report

Merging #4632 (80f670a) into master (9607554) will decrease coverage by 0.6400%.
The diff coverage is 39.9056%.

Flag	Coverage Δ
cdc	`59.4522% <45.5122%> (-0.4700%)`	⬇️
dm	`51.2830% <32.2139%> (-0.7458%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@               Coverage Diff                @@
##             master      #4632        +/-   ##
================================================
- Coverage   55.6402%   55.0001%   -0.6401%     
================================================
  Files           494        512        +18     
  Lines         61283      63638      +2355     
================================================
+ Hits          34098      35001       +903     
- Misses        23750      25154      +1404     
- Partials       3435       3483        +48

Signed-off-by: Neil Shen <[email protected]>

sdojjy · 2022-02-23T02:37:10Z

cdc/sorter/leveldb/compactor.go

-		return false
+// trySchdeduleCompact try to schedule a compact task.
+// Returns true if it schedules compact task successfully.
+func (s *CompactScheduler) trySchdeduleCompact(id actor.ID, deleteCount int) bool {


Suggested change

func (s *CompactScheduler) trySchdeduleCompact(id actor.ID, deleteCount int) bool {

func (s *CompactScheduler) tryScheduleCompact(id actor.ID, deleteCount int) bool {

sdojjy · 2022-02-23T02:38:42Z

cdc/sorter/leveldb/leveldb.go

@@ -138,6 +138,14 @@ func (ldb *DBActor) close(err error) {
 	ldb.closedWg.Done()
 }

+func (ldb *DBActor) trySchdeduleCompact() {


Suggested change

func (ldb *DBActor) trySchdeduleCompact() {

func (ldb *DBActor) tryScheduleCompact() {

Signed-off-by: Neil Shen <[email protected]>

sdojjy · 2022-02-23T02:53:16Z

cdc/sorter/leveldb/leveldb.go

+			if len(task.DeleteReq.Range[0]) != 0 && len(task.DeleteReq.Range[1]) != 0 {
+				// Force write pending write batch before delete range.
+				if err := ldb.maybeWrite(true); err != nil {
+					log.Panic("db error", zap.Error(err))


log tableID?

Signed-off-by: Neil Shen <[email protected]>

zhaoxinyu · 2022-02-23T02:59:31Z

cdc/sorter/leveldb/table_sorter.go

-// CleanupTask returns a clean up task that delete sorter's data.
-func (ls *Sorter) CleanupTask() actormsg.Message {
-	return actormsg.SorterMessage(message.NewCleanupTask(ls.uid, ls.tableID))
+// CleanupFunc returns a funcation that cleans up sorter's data.


Suggested change

// CleanupFunc returns a funcation that cleans up sorter's data.

// CleanupFunc returns a function that cleans up sorter's data.

zhaoxinyu · 2022-02-23T03:00:02Z

cdc/sorter/leveldb/table_sorter.go

+	return func(ctx context.Context) error {
+		task := message.Task{UID: ls.uid, TableID: ls.tableID}
+		task.DeleteReq = &message.DeleteRequest{
+			// We do not set task.Delte.Count, because we don't know


Suggested change

// We do not set task.Delte.Count, because we don't know

// We do not set task.Delete.Count, because we don't know

zhaoxinyu · 2022-02-23T03:10:31Z

pkg/config/sorter.go

@@ -48,8 +48,8 @@ func (c *SorterConfig) ValidateAndAdjust() error {
 	if c.NumWorkerPoolGoroutine < 1 {
 		return cerror.ErrIllegalSorterParameter.GenWithStackByArgs("num-workerpool-goroutine should be at least 1, larger than 8 is recommended")
 	}
-	if c.MaxMemoryPressure < 0 || c.MaxMemoryPressure > 100 {
-		return cerror.ErrIllegalSorterParameter.GenWithStackByArgs("max-memory-percentage should be a percentage")
+	if c.MaxMemoryPressure <= 0 || c.MaxMemoryPressure > 80 {


Should this SorterConfig. MaxMemoryPressure field be renamed as MaxMemoryPercentage?

sdojjy · 2022-02-23T03:12:31Z

cdc/sorter/leveldb/leveldb.go

@@ -229,6 +231,20 @@ func (ldb *DBActor) Poll(ctx context.Context, tasks []actormsg.Message) bool {
 				log.Panic("db error", zap.Error(err))
 			}
 		}
+		if task.DeleteReq != nil {
+			ldb.deleteCount += task.DeleteReq.Count


for DBActor, task.DeleteReq.Count is always 0? the value is from CleanFunc

For now, it's always zero, we will fill Count in later PRs.

sdojjy · 2022-02-23T03:18:39Z

cdc/sorter/leveldb/leveldb.go

+					log.Panic("db error", zap.Error(err))
+				}
+				start, end := task.DeleteReq.Range[0], task.DeleteReq.Range[1]
+				if err := ldb.db.DeleteRange(start, end); err != nil {


is DeleteRange a blocking function call or non-blocking function call?

Signed-off-by: Neil Shen <[email protected]>

sdojjy

lgtm

zhaoxinyu · 2022-02-23T03:28:15Z

/LGTM

overvenus · 2022-02-23T05:02:39Z

/merge

ti-chi-bot · 2022-02-23T05:02:41Z

This pull request has been accepted and is ready to merge.

Commit hash: c39e522

overvenus · 2022-02-23T12:24:55Z

/run-integration-test

overvenus · 2022-02-23T12:29:06Z

/run-check-issue-triage-complete

overvenus · 2022-02-23T13:10:33Z

/merge

overvenus · 2022-02-24T02:58:43Z

/merge

ti-chi-bot · 2022-02-24T02:59:00Z

@overvenus: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

sorter/leveldb(ticdc): replace cleaner with delete range

02b893f

* Add DeleteRange API to DB * Update DB dashboard in grafana * Adjust DB block cahce by sorter.max-memory-percentage Signed-off-by: Neil Shen <[email protected]>

overvenus added subject/performance Denotes an issue or pull request is related to replication performance. component/sorter Sorter component. labels Feb 19, 2022

ti-chi-bot added do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 19, 2022

overvenus added 2 commits February 20, 2022 11:10

add block cache hit ratio metrics

642514e

Signed-off-by: Neil Shen <[email protected]>

Merge branch 'master' into leveldb-delete-range

116108d

Signed-off-by: Neil Shen <[email protected]>

sdojjy reviewed Feb 23, 2022

View reviewed changes

address comments

a2e8488

Signed-off-by: Neil Shen <[email protected]>

sdojjy reviewed Feb 23, 2022

View reviewed changes

overvenus added 2 commits February 23, 2022 10:58

address comments

9fa2a53

Signed-off-by: Neil Shen <[email protected]>

fix typo

bfe4988

Signed-off-by: Neil Shen <[email protected]>

zhaoxinyu reviewed Feb 23, 2022

View reviewed changes

sdojjy reviewed Feb 23, 2022

View reviewed changes

address comments

c39e522

Signed-off-by: Neil Shen <[email protected]>

sdojjy approved these changes Feb 23, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 23, 2022

zhaoxinyu approved these changes Feb 23, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 23, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 23, 2022

Merge branch 'master' into leveldb-delete-range

ab4e13c

overvenus added the type/enhancement The issue or PR belongs to an enhancement. label Feb 23, 2022

ti-chi-bot removed the do-not-merge/needs-triage-completed label Feb 23, 2022

ti-chi-bot added 2 commits February 23, 2022 21:10

Merge branch 'master' into leveldb-delete-range

8d28344

Merge branch 'master' into leveldb-delete-range

80f670a

Merge branch 'master' into leveldb-delete-range

94835ce

ti-chi-bot merged commit c396df1 into pingcap:master Feb 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sorter/leveldb(ticdc): replace cleaner with delete range #4632

sorter/leveldb(ticdc): replace cleaner with delete range #4632

overvenus commented Feb 19, 2022

ti-chi-bot commented Feb 19, 2022 •

edited

Loading

codecov-commenter commented Feb 19, 2022 •

edited

Loading

sdojjy Feb 23, 2022

sdojjy Feb 23, 2022

sdojjy Feb 23, 2022

zhaoxinyu Feb 23, 2022

zhaoxinyu Feb 23, 2022

zhaoxinyu Feb 23, 2022 •

edited

Loading

sdojjy Feb 23, 2022 •

edited

Loading

overvenus Feb 23, 2022

sdojjy Feb 23, 2022

sdojjy left a comment

zhaoxinyu commented Feb 23, 2022

overvenus commented Feb 23, 2022

ti-chi-bot commented Feb 23, 2022

overvenus commented Feb 23, 2022

overvenus commented Feb 23, 2022

overvenus commented Feb 23, 2022

overvenus commented Feb 24, 2022

ti-chi-bot commented Feb 24, 2022

	func (s *CompactScheduler) trySchdeduleCompact(id actor.ID, deleteCount int) bool {
	func (s *CompactScheduler) tryScheduleCompact(id actor.ID, deleteCount int) bool {

	func (ldb *DBActor) trySchdeduleCompact() {
	func (ldb *DBActor) tryScheduleCompact() {

	// CleanupFunc returns a funcation that cleans up sorter's data.
	// CleanupFunc returns a function that cleans up sorter's data.

	// We do not set task.Delte.Count, because we don't know
	// We do not set task.Delete.Count, because we don't know

sorter/leveldb(ticdc): replace cleaner with delete range #4632

sorter/leveldb(ticdc): replace cleaner with delete range #4632

Conversation

overvenus commented Feb 19, 2022

What problem does this PR solve?

Check List

Release note

ti-chi-bot commented Feb 19, 2022 • edited Loading

codecov-commenter commented Feb 19, 2022 • edited Loading

Codecov Report

sdojjy Feb 23, 2022

Choose a reason for hiding this comment

sdojjy Feb 23, 2022

Choose a reason for hiding this comment

sdojjy Feb 23, 2022

Choose a reason for hiding this comment

zhaoxinyu Feb 23, 2022

Choose a reason for hiding this comment

zhaoxinyu Feb 23, 2022

Choose a reason for hiding this comment

zhaoxinyu Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

sdojjy Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

overvenus Feb 23, 2022

Choose a reason for hiding this comment

sdojjy Feb 23, 2022

Choose a reason for hiding this comment

sdojjy left a comment

Choose a reason for hiding this comment

zhaoxinyu commented Feb 23, 2022

overvenus commented Feb 23, 2022

ti-chi-bot commented Feb 23, 2022

overvenus commented Feb 23, 2022

overvenus commented Feb 23, 2022

overvenus commented Feb 23, 2022

overvenus commented Feb 24, 2022

ti-chi-bot commented Feb 24, 2022

ti-chi-bot commented Feb 19, 2022 •

edited

Loading

codecov-commenter commented Feb 19, 2022 •

edited

Loading

zhaoxinyu Feb 23, 2022 •

edited

Loading

sdojjy Feb 23, 2022 •

edited

Loading