Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

min-safe-ts: fix MinSafeTS might be set to MaxUint64 permanently #999

Merged
merged 4 commits into from
Nov 21, 2023

Conversation

HuSharp
Copy link
Member

@HuSharp HuSharp commented Oct 9, 2023

related #991
cherry-pick #994

The core question is the getter

  • we introduce PD API to not execute go func which for KV request, resulting in not updating safeTSMap.
    • updateMinSafeTS relies on safeTSMap which makes sense(because actually, we can call updateMinSafeTS to kvReuqestUpdater[to indicate func base]).
  • And we need to update minsafeTS to make sure when API fails we can fall back to the original way which is by kv request.
  • But the core problem is: updateMinSafeTS will return maxUnit64 when the first kv request returns 0 and then although PD API returns correctly[maybe kv is not initialized], TS can not change maxUnit64.
    • to resolve this question, we need to regard maxUnit64 as 0 which means there is an initial state.

Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
Copy link
Member Author

HuSharp commented Oct 9, 2023

@JmPotato @nolouch PTAL as well, thx!

@HuSharp
Copy link
Member Author

HuSharp commented Nov 9, 2023

@JmPotato friendly ping :)

Copy link
Member

@JmPotato JmPotato left a comment

Choose a reason for hiding this comment

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

Please follow the latest pd_api_test.go to unify the test.

Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
Copy link
Member Author

HuSharp commented Nov 15, 2023

Please follow the latest pd_api_test.go to unify the test.

done.

@HuSharp
Copy link
Member Author

HuSharp commented Nov 20, 2023

@JmPotato friendly ping :)

@HuSharp HuSharp force-pushed the fix_cluster_init_zero_6.5 branch 2 times, most recently from 16c4ce4 to 7bf04ba Compare November 21, 2023 02:16
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@disksing disksing merged commit c1041a4 into tikv:tidb-6.5 Nov 21, 2023
7 of 9 checks passed
@HuSharp HuSharp deleted the fix_cluster_init_zero_6.5 branch November 21, 2023 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants