Skip to content

concurrency_manager: check update_max_ts against a limit (#17917)#18048

Merged
ti-chi-bot[bot] merged 6 commits intotikv:release-7.1from
ti-chi-bot:cherry-pick-17917-to-release-7.1
Jan 8, 2025
Merged

concurrency_manager: check update_max_ts against a limit (#17917)#18048
ti-chi-bot[bot] merged 6 commits intotikv:release-7.1from
ti-chi-bot:cherry-pick-17917-to-release-7.1

Conversation

@ti-chi-bot
Copy link
Member

@ti-chi-bot ti-chi-bot commented Dec 24, 2024 •
edited by ekexium
Loading

This is an automated cherry-pick of #17917

What is changed and how it works?

Issue Number: close #17916

What's Changed:

concurrency_manager: add safety boundary for max_ts updates

For this cherry-pick: only log error, do not return error or panic.

Add `max_ts_limit` to prevent unreasonable timestamp updates. The limit is 
synchronized with PD timestamp periodically.

Updates from PD bypass this limit.

Metric:
image

Benchmark of update_max_ts:
master

update_max_ts           time:   [3.7480 ns 3.7481 ns 3.7483 ns]

This PR

update_max_ts           time:   [4.1907 ns 4.1909 ns 4.1911 ns]

Read operations typically need hundreds of microseconds, so I suppose this regression has little impact.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

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

Release note

Introduce a max-ts checker.

@ti-chi-bot ti-chi-bot added dco-signoff: yes Indicates the PR's author has signed the dco. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. type/cherry-pick-for-release-7.1 This PR is cherry-picked to release-7.1 from a source PR. labels Dec 24, 2024
@ti-chi-bot ti-chi-bot mentioned this pull request Dec 24, 2024
Merged
9 tasks
@ekexium
cherry pick tikv#17917: remove configs and only log errors
a7ced27 
Signed-off-by: ekexium <eke@fastmail.com>
@ekexium ekexium force-pushed the cherry-pick-17917-to-release-7.1 branch from ef6f839 to a7ced27 Compare December 24, 2024 14:15
@ekexium
Copy link
Contributor

ekexium commented Dec 24, 2024

/retest

@ekexium ekexium requested review from MyonKeminta and you06 December 24, 2024 15:09
ekexium and others added 2 commits January 7, 2025 13:59
@ekexium
concurrency_manager: double check via PD TSO before reporting error o…
4e77e00 
…f invalid max-ts update (tikv#18057)

close tikv#18055

concurrency_manager: double check via PD TSO before reporting error of invalid max-ts update

Signed-off-by: ekexium <eke@fastmail.com>
@ekexium @ti-chi-bot
concurrency_manager: make max-ts checker more robust (tikv#18080)
09a9f72 
ref tikv#18055

When validating max-ts updates, do not report error or panic unless confirmed by PD TSO.
This reduces both false positive and false negative cases.

Signed-off-by: ekexium <eke@fastmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
@ti-chi-bot ti-chi-bot bot added cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Jan 8, 2025
@ekexium
reword logs
b60b3ed 
Signed-off-by: ekexium <eke@fastmail.com>
@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jan 8, 2025
@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 8, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 8, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-01-08 06:35:18.845685157 +0000 UTC m=+335462.134516862: ☑️ agreed by you06.
  • 2025-01-08 07:21:36.727130252 +0000 UTC m=+338240.015961951: ☑️ agreed by MyonKeminta.

@ekexium
Copy link
Contributor

ekexium commented Jan 8, 2025

/hold

@ti-chi-bot ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 8, 2025
@ekexium
Copy link
Contributor

ekexium commented Jan 8, 2025

/unhold

@ti-chi-bot ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 8, 2025
@ekexium ekexium force-pushed the cherry-pick-17917-to-release-7.1 branch from 3d2b360 to 6ec935f Compare January 8, 2025 07:54
@ekexium
remove online change
8d8d40a 
Signed-off-by: ekexium <eke@fastmail.com>
@ekexium ekexium force-pushed the cherry-pick-17917-to-release-7.1 branch from 6ec935f to 8d8d40a Compare January 8, 2025 12:18
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 8, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MyonKeminta, you06

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added the approved label Jan 8, 2025
@ekexium
Copy link
Contributor

ekexium commented Jan 8, 2025

/retest

@ti-chi-bot ti-chi-bot bot merged commit 12a47f4 into tikv:release-7.1 Jan 8, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@you06 you06 you06 approved these changes

@MyonKeminta MyonKeminta MyonKeminta approved these changes

Assignees

@ekexium ekexium

Labels

approved cherry-pick-approved Cherry pick PR approved by release team. dco-signoff: yes Indicates the PR's author has signed the dco. lgtm release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. type/cherry-pick-for-release-7.1 This PR is cherry-picked to release-7.1 from a source PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ti-chi-bot @ekexium @you06 @MyonKeminta