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

pd http: support api to get store min resolved ts #793

Merged
merged 10 commits into from
May 12, 2023

Conversation

HuSharp
Copy link
Member

@HuSharp HuSharp commented May 10, 2023

Close #795
after tikv/pd#6413 merged, We can replace resolved-ts's current implementation in client-go

  • support api mechanism to pd
  • support api to get store min resolved ts

@HuSharp HuSharp force-pushed the add_store_min_resolved_ts branch from 88d7fa8 to acb6745 Compare May 10, 2023 08:02
Signed-off-by: husharp <[email protected]>
@HuSharp HuSharp force-pushed the add_store_min_resolved_ts branch from acb6745 to 1648fc9 Compare May 10, 2023 08:03
Signed-off-by: husharp <[email protected]>
Signed-off-by: husharp <[email protected]>
tikv/kv.go Outdated Show resolved Hide resolved
tikv/kv.go Outdated Show resolved Hide resolved
util/pd.go Outdated Show resolved Hide resolved
tikv/kv.go Outdated Show resolved Hide resolved
util/pd.go Outdated Show resolved Hide resolved
util/pd.go Outdated Show resolved Hide resolved
util/pd.go Show resolved Hide resolved
util/pd.go Outdated Show resolved Hide resolved
util/pd.go Outdated Show resolved Hide resolved
util/pd_test.go Show resolved Hide resolved
@HuSharp HuSharp marked this pull request as ready for review May 11, 2023 06:19
Signed-off-by: husharp <[email protected]>
@HuSharp HuSharp force-pushed the add_store_min_resolved_ts branch from 036d5b5 to b2d9fe6 Compare May 11, 2023 06:55
integration_tests/raw/api_test.go Outdated Show resolved Hide resolved
return c.Client.CloseAddr(addr)
}

func (s *apiTestSuite) TestGetStoreMinResolvedTS() {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's proper to put resolved_ts test inside integration_tests/raw/api_test.go since it's for the RawKV API testing.

Copy link
Member Author

Choose a reason for hiding this comment

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

move to pd_api_test file

tikv/kv.go Outdated Show resolved Hide resolved
tikv/kv.go Outdated Show resolved Hide resolved
Signed-off-by: husharp <[email protected]>
Signed-off-by: husharp <[email protected]>
@HuSharp
Copy link
Member Author

HuSharp commented May 11, 2023

@Defined2014 PTAL, thx!

integration_tests/pd_api_test.go Outdated Show resolved Hide resolved
integration_tests/pd_api_test.go Outdated Show resolved Hide resolved
integration_tests/pd_api_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Defined2014 Defined2014 left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: husharp <[email protected]>
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.

The rest LGTM.

return
// Try to get the minimum resolved timestamp of the store from PD.
if s.pdHttpClient != nil {
safeTS, err = s.pdHttpClient.GetStoreMinResolvedTS(ctx, storeID)
Copy link
Member

Choose a reason for hiding this comment

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

We could add a debug log to output safeTS and err when it's not nil.

Signed-off-by: husharp <[email protected]>
@HuSharp HuSharp force-pushed the add_store_min_resolved_ts branch from e708d8a to 6e4bfa5 Compare May 11, 2023 09:37
@disksing disksing merged commit adb48af into tikv:master May 12, 2023
disksing pushed a commit that referenced this pull request May 16, 2023
iosmanthus added a commit that referenced this pull request May 22, 2023
Co-authored-by: MyonKeminta <[email protected]>
Co-authored-by: disksing <[email protected]>
Co-authored-by: MyonKeminta <[email protected]>
Co-authored-by: Violin <[email protected]>
Co-authored-by: Smilencer <[email protected]>
Co-authored-by: you06 <[email protected]>
Co-authored-by: Hu# <[email protected]>
Co-authored-by: Connor <[email protected]>
Co-authored-by: zyguan <[email protected]>
fix case typo in comment. (#778)
fix goroutine leak (#784)
fix TestRURuntimeStatsCleanUp (#787)
Fix wrong resource group name for some requests (#788)
resolver: support verifying primary for check_txn_status (#777)
resolver: handle pessimistic locks in BatchResolveLocks (#794)
resolved ts  (#793)
ResolveLocks for unistore (#807)
@HuSharp HuSharp deleted the add_store_min_resolved_ts branch October 11, 2023 04:53
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.

minResolvedTS: use pd API to fetch the store resolved_ts firstly
4 participants