*: support renew lease for read operation on cached table #29840

JayLZhou · 2021-11-16T09:25:38Z

What problem does this PR solve?

Issue Number: #29849

Problem Summary:
This is a subtask of #25293

We need to implement lease renewal for the read cache table.
for example:

startTs --> 2s
readLease ---> 5s
..........
nowTs 4s
we should renew readLease ---> 4s + 3s = 7s

What is changed and how it works?

Check List

Tests

Unit test

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

ti-chi-bot · 2021-11-16T09:25:39Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lcwangchao
tiancaiamao

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.

infoschema/builder.go

infoschema/infoschema.go

table/tables/cache.go

table/tables/cache_test.go

tiancaiamao · 2021-11-17T12:48:45Z

table/tables/state_remote.go

+func (r *mockStateRemoteHandle) RenewLease(tid int64, ts uint64, op RenewLeaseType) (bool, error) {
+	switch op {
+	case RenewReadLease:
+		op := &renewLeaseForReadOP{tid: tid, ts: ts}


Renew lease need to check the lock still hold the lease...

domain/domain.go

infoschema/builder.go

table/tables/cache.go

table/tables/state_remote.go

hawkingrei · 2021-11-21T09:51:45Z

domain/domain.go

 	go do.topNSlowQueryLoop()
 	go do.infoSyncerKeeper()
+	go do.renewLease()


now you have used the var wg util.WaitGroupWrapper and you can simplify the code like here.

tidb/br/pkg/lightning/restore/checksum_test.go

Line 100 in b266d7d

var wg util.WaitGroupWrapper

you can only change the renewLease for using the Run of WaitGroupWrapper.

the other leave it as it is because WaitGroupWrapper is compatible with WaitGroup.

table/tables/cache.go

tiancaiamao

Renew lease need two parameter: the ts now, and the lease value it want to set to.
Compare ts now in the local and set it is not a safe operation...

JayLZhou · 2021-11-22T06:28:23Z

/run-check_dev_2

lcwangchao · 2021-11-22T06:31:22Z

domain/domain.go

@@ -1729,6 +1730,22 @@ func (do *Domain) MockInfoCacheAndLoadInfoSchema(is infoschema.InfoSchema) {
 	do.infoCache.Insert(is, 0)
 }

+func (do *Domain) renewLease() {


why a global gorountine

JayLZhou · 2021-11-22T06:39:47Z

/run-check_dev_2

lcwangchao · 2021-11-22T10:40:42Z

/merge

ti-chi-bot · 2021-11-22T10:40:45Z

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

Commit hash: 2000cc1

JayLZhou · 2021-11-22T11:17:42Z

/run-check_dev_2

ti-chi-bot · 2021-11-22T11:33:59Z

@jayl-zxl: 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.

JayLZhou · 2021-11-22T12:15:32Z

/run-check_dev_2

JayLZhou · 2021-11-22T12:34:53Z

/run-check_dev_2

JayLZhou · 2021-11-22T12:43:08Z

/run-check_dev_2

JayLZhou · 2021-11-22T14:28:13Z

/run-check_dev_2

draft_renew_read

7dd848e

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 16, 2021

JayLZhou marked this pull request as draft November 16, 2021 09:25

ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 16, 2021

bug_leak

332c11a

tiancaiamao reviewed Nov 17, 2021

View reviewed changes

JayLZhou added 2 commits November 17, 2021 21:03

bug

ea991a2

save_bug

7734a44

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 18, 2021

draft_1

2485db3

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 19, 2021

JayLZhou added 4 commits November 19, 2021 21:50

draft_1

3024cf4

draft_1

73c4050

draft_1

3a35566

draft_1

1531867

JayLZhou marked this pull request as ready for review November 19, 2021 14:43

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 19, 2021

JayLZhou added 2 commits November 19, 2021 23:04

draft_1

cacd897

draft_1

f4abff4

JayLZhou requested a review from tiancaiamao November 19, 2021 15:40

hawkingrei reviewed Nov 20, 2021

View reviewed changes

domain/domain.go Outdated Show resolved Hide resolved

domain/domain.go Outdated Show resolved Hide resolved

domain/domain.go Outdated Show resolved Hide resolved

tiancaiamao reviewed Nov 21, 2021

View reviewed changes

JayLZhou added 3 commits November 21, 2021 17:03

draft_1

c9bad74

draft_1

2934379

draft_1

adb83cb

hawkingrei reviewed Nov 21, 2021

View reviewed changes

tiancaiamao reviewed Nov 22, 2021

View reviewed changes

table/tables/cache.go Outdated Show resolved Hide resolved

tiancaiamao reviewed Nov 22, 2021

View reviewed changes

table/tables/cache.go Show resolved Hide resolved

tiancaiamao reviewed Nov 22, 2021

View reviewed changes

JayLZhou added 2 commits November 22, 2021 13:46

solve_comment

a6f8369

solve_comment

fedf01c

tiancaiamao approved these changes Nov 22, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 22, 2021

solve_comment

2000cc1

lcwangchao reviewed Nov 22, 2021

View reviewed changes

JayLZhou requested review from lcwangchao, tiancaiamao and hawkingrei November 22, 2021 07:24

lcwangchao approved these changes Nov 22, 2021

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 Nov 22, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 22, 2021

Merge branch 'master' into renew_read

0e575c5

Merge branch 'master' into renew_read

48b41f1

ti-chi-bot merged commit c48db39 into pingcap:master Nov 22, 2021

JayLZhou deleted the renew_read branch November 22, 2021 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: support renew lease for read operation on cached table #29840

*: support renew lease for read operation on cached table #29840

JayLZhou commented Nov 16, 2021 •

edited

Loading

ti-chi-bot commented Nov 16, 2021 •

edited

Loading

tiancaiamao Nov 17, 2021

hawkingrei Nov 21, 2021

hawkingrei Nov 21, 2021 •

edited

Loading

hawkingrei Nov 21, 2021

tiancaiamao left a comment

JayLZhou commented Nov 22, 2021

lcwangchao Nov 22, 2021

JayLZhou commented Nov 22, 2021

lcwangchao commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

*: support renew lease for read operation on cached table #29840

*: support renew lease for read operation on cached table #29840

Conversation

JayLZhou commented Nov 16, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Nov 16, 2021 • edited Loading

tiancaiamao Nov 17, 2021

Choose a reason for hiding this comment

hawkingrei Nov 21, 2021

Choose a reason for hiding this comment

hawkingrei Nov 21, 2021 • edited Loading

Choose a reason for hiding this comment

hawkingrei Nov 21, 2021

Choose a reason for hiding this comment

tiancaiamao left a comment

Choose a reason for hiding this comment

JayLZhou commented Nov 22, 2021

lcwangchao Nov 22, 2021

Choose a reason for hiding this comment

JayLZhou commented Nov 22, 2021

lcwangchao commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

ti-chi-bot commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

JayLZhou commented Nov 22, 2021

JayLZhou commented Nov 16, 2021 •

edited

Loading

ti-chi-bot commented Nov 16, 2021 •

edited

Loading

hawkingrei Nov 21, 2021 •

edited

Loading