resolved_ts: return early after regions already had quorum by 5kbpers · Pull Request #11352 · tikv/tikv

5kbpers · 2021-11-12T04:40:34Z

Signed-off-by: 5kbpers tangminghua@pingcap.com

What problem does this PR solve?

Issue Number: close #11351

Problem Summary:

What is changed and how it works?

What's Changed:

export region_resolved_ts_store from resolved_ts and import it in cdc
use select_all instead of join_all for returning early

Related changes

Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test

Release note

Fix resolved ts lag increased after stoping a tikv

ti-chi-bot · 2021-11-12T04:40:36Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

NingLin-P
overvenus

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.

Details

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

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

5kbpers · 2021-11-19T07:39:57Z

/run-all-tests

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

NingLin-P

LGTM

0xPoe · 2021-11-19T09:11:59Z

/run-integration-cdc-test

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

0xPoe · 2021-11-19T10:33:03Z

/run-integration-cdc-test

components/resolved_ts/src/advance.rs

liuzix · 2021-11-20T03:23:08Z

components/resolved_ts/src/advance.rs

+        })
+        .collect();
+    for _ in 0..store_count {
+        let (res, _, remains) = select_all(stores).await;


Can you add comments on why using select_all as opposed to the original solution using join_all?

components/resolved_ts/src/advance.rs

liuzix · 2021-11-20T03:26:51Z

components/resolved_ts/src/advance.rs

+    let leader_info_size = store_map
+        .values()
+        .next()
+        .map_or(0, |regions| regions[0].compute_size());


Can there be 0 regions?

liuzix · 2021-11-20T03:27:20Z

components/resolved_ts/src/advance.rs

+    // region_id -> peers id, record the responses
+    let mut resp_map: HashMap<u64, Vec<u64>> = HashMap::default();
+    // region_id -> `(Vec<Peer>, LeaderInfo)`
+    let info_map = region_read_progress.dump_leader_infos(&regions);


How expensive is this function?

O(n), n is the number of regions

Signed-off-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: Zixiong Liu <liuzixiong@pingcap.com>

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

overvenus · 2021-11-24T07:31:35Z

components/resolved_ts/src/advance.rs

+        let cb = ChannelBuilder::new(env.clone());
+        let channel = security_mgr.connect(cb, &store.address);
+        tikv_clients
+            .lock()
+            .unwrap()
+            .insert(store_id, TikvClient::new(channel));


When do we release these connections?

There is an issue #11400 about it, I think we can fix it in another PR.

After some requests failed.

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

5kbpers · 2021-11-24T09:05:30Z

/run-all-tests

overvenus

LGTM

5kbpers · 2021-11-24T09:23:08Z

/merge

ti-chi-bot · 2021-11-24T09:23:09Z

@5kbpers: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and 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.

If you have any questions about the PR merge process, please refer to pr process.

Details

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.

ti-chi-bot · 2021-11-24T09:23:11Z

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

Details

Commit hash: a542652

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2021-12-02T04:04:32Z

cherry pick to release-5.3 in PR #11537

ti-srebot · 2021-12-02T04:04:57Z

cherry pick to release-5.0 in PR #11538

ti-srebot · 2021-12-02T04:05:21Z

cherry pick to release-5.2 in PR #11539

ti-srebot · 2021-12-02T04:05:49Z

cherry pick to release-5.1 in PR #11540

* close tikv#11351 Signed-off-by: 5kbpers <tangminghua@pingcap.com> * fix tests Signed-off-by: 5kbpers <tangminghua@pingcap.com> * fix tests Signed-off-by: 5kbpers <tangminghua@pingcap.com> * Apply suggestions from code review Signed-off-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: Zixiong Liu <liuzixiong@pingcap.com> * address comments Signed-off-by: 5kbpers <tangminghua@pingcap.com> * make clippy happy Signed-off-by: 5kbpers <tangminghua@pingcap.com> * close tikv#11400 Signed-off-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: qupeng <qupeng@pingcap.com> Co-authored-by: Zixiong Liu <liuzixiong@pingcap.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

* resolved_ts: return early after regions already had quorum (#11352) * close #11351 Signed-off-by: 5kbpers <tangminghua@pingcap.com> * fix tests Signed-off-by: 5kbpers <tangminghua@pingcap.com> * fix tests Signed-off-by: 5kbpers <tangminghua@pingcap.com> * Apply suggestions from code review Signed-off-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: Zixiong Liu <liuzixiong@pingcap.com> * address comments Signed-off-by: 5kbpers <tangminghua@pingcap.com> * make clippy happy Signed-off-by: 5kbpers <tangminghua@pingcap.com> * close #11400 Signed-off-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: qupeng <qupeng@pingcap.com> Co-authored-by: Zixiong Liu <liuzixiong@pingcap.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> * Support to iterate table properties for KvEngine (#11388) * support to iterate table properties for KvEngine * close #11387 Signed-off-by: qupeng <qupeng@pingcap.com> * cdc: introduce TsFilter into incremental scan (#11385) * close #11384 Signed-off-by: qupeng <qupeng@pingcap.com> * support to get approximate keys and size for a given range from UserCollectedProperties (#11465) Signed-off-by: qupeng <qupeng@pingcap.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> * cherry pick pr #11615 Signed-off-by: qupeng <qupeng@pingcap.com> Co-authored-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: Zixiong Liu <liuzixiong@pingcap.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

…11538) * ref #11351 Signed-off-by: 5kbpers <tangminghua@pingcap.com>

… (tikv#11538) * ref tikv#11351 Signed-off-by: 5kbpers <tangminghua@pingcap.com> Signed-off-by: Yilin Chen <sticnarf@gmail.com>

…11540) close #11351, ref #11352 Signed-off-by: 5kbpers <tangminghua@pingcap.com> Signed-off-by: Yilin Chen <sticnarf@gmail.com> Co-authored-by: qupeng <qupeng@pingcap.com>

…11537) close #11351, ref #11352 Signed-off-by: ti-srebot <ti-srebot@pingcap.com> Signed-off-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

…11539) close #11351, ref #11352 Signed-off-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: 5kbpers <tangminghua@pingcap.com>

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 12, 2021

5kbpers and others added 2 commits November 12, 2021 12:40

close tikv#11351

9521c15

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

Merge branch 'master' into fix-resolved-ts

a61f47b

5kbpers added 2 commits November 19, 2021 15:40

Merge branch 'master' into fix-resolved-ts

ed0b7c3

fix tests

dbb7d6d

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

5kbpers force-pushed the fix-resolved-ts branch from 479033c to dbb7d6d Compare November 19, 2021 08:45

NingLin-P approved these changes Nov 19, 2021

View reviewed changes

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

fix tests

a1d3253

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

liuzix reviewed Nov 20, 2021

View reviewed changes

5kbpers and others added 3 commits November 23, 2021 14:05

Apply suggestions from code review

24c7d42

Signed-off-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: Zixiong Liu <liuzixiong@pingcap.com>

address comments

4634b6d

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

make clippy happy

f38bd35

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

overvenus reviewed Nov 24, 2021

View reviewed changes

5kbpers added 2 commits November 24, 2021 16:31

close tikv#11400

f75b5df

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

Merge branch 'master' into fix-resolved-ts

a542652

overvenus approved these changes Nov 24, 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 24, 2021

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

Merge branch 'master' into fix-resolved-ts

8a5c4d3

ti-srebot pushed a commit to ti-srebot/tikv that referenced this pull request Dec 2, 2021

cherry pick tikv#11352 to release-5.3

31734f5

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Dec 2, 2021

resolved_ts: return early after regions already had quorum (#11352) #11537

Merged

ti-srebot mentioned this pull request Dec 2, 2021

resolved_ts: return early after regions already had quorum (#11352) #11538

Merged

ti-srebot mentioned this pull request Dec 2, 2021

resolved_ts: return early after regions already had quorum (#11352) #11539

Merged

ti-srebot mentioned this pull request Dec 2, 2021

resolved_ts: return early after regions already had quorum (#11352) #11540

Merged

amyangfei mentioned this pull request Dec 9, 2021

The abnormal changefeed impact other normal changefeed to sync task pingcap/tiflow#1708

Closed

hicqu deleted the fix-resolved-ts branch December 10, 2021 07:22

hicqu mentioned this pull request Dec 10, 2021

cdc: pick some fixes from master #11632

Merged

zhouqiang-cl pushed a commit that referenced this pull request Dec 21, 2021

resolved_ts: return early after regions already had quorum (#11352) (#…

d81e32a

…11538) * ref #11351 Signed-off-by: 5kbpers <tangminghua@pingcap.com>

NingLin-P mentioned this pull request Jan 7, 2022

after two tikv restart， The “Max Resolved TS gap” display is unreasonable #10702

Closed

NingLin-P mentioned this pull request Feb 7, 2022

Remove StoreMeta.leaders since it is not use anymore #11933

Closed

This was referenced Feb 27, 2022

releases: add tidb 5.3.1 release notes pingcap/docs#7699

Merged

add 5.3.1 release notes pingcap/docs-cn#8495

Merged

5kbpers pushed a commit to ti-srebot/tikv that referenced this pull request Apr 14, 2022

cherry pick tikv#11352 to release-5.2

dbbb75b

Signed-off-by: 5kbpers <tangminghua@pingcap.com>

ti-chi-bot pushed a commit that referenced this pull request Apr 14, 2022

resolved_ts: return early after regions already had quorum (#11352) (#…

ec93f55

…11539) close #11351, ref #11352 Signed-off-by: 5kbpers <tangminghua@pingcap.com> Co-authored-by: 5kbpers <tangminghua@pingcap.com>

Conversation

5kbpers commented Nov 12, 2021

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

ti-chi-bot commented Nov 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

5kbpers commented Nov 19, 2021

Uh oh!

NingLin-P left a comment

Choose a reason for hiding this comment

Uh oh!

0xPoe commented Nov 19, 2021

Uh oh!

0xPoe commented Nov 19, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

5kbpers commented Nov 24, 2021

Uh oh!

overvenus left a comment

Choose a reason for hiding this comment

Uh oh!

5kbpers commented Nov 24, 2021

Uh oh!

ti-chi-bot commented Nov 24, 2021

Uh oh!

ti-chi-bot commented Nov 24, 2021

Uh oh!

ti-srebot commented Dec 2, 2021

Uh oh!

ti-srebot commented Dec 2, 2021

Uh oh!

ti-srebot commented Dec 2, 2021

Uh oh!

ti-srebot commented Dec 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ti-chi-bot commented Nov 12, 2021 •

edited

Loading