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.

Sign up for GitHub

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

Jump to bottom

MinSafeTS support TiFlash #642

Merged
merged 8 commits into from
Dec 19, 2022
Merged

MinSafeTS support TiFlash #642

merged 8 commits into from
Dec 19, 2022

Conversation

hehechen
Copy link
Contributor

Signed-off-by: hehechen [email protected]
MinSafeTS support TiFlash to support TiFlash stale read.

@hehechen
Copy link
Contributor Author

/cc @sticnarf @Connor1996

@hehechen
Copy link
Contributor Author

/run-all-tests

JmPotato
JmPotato reviewed Dec 15, 2022
View reviewed changes
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.

Could we add a test for this change also? The rest LGTM.

tikv/kv.go Outdated Show resolved Hide resolved
lhy1024
lhy1024 reviewed Dec 15, 2022
View reviewed changes
Copy link

@lhy1024 lhy1024 left a comment

Choose a reason for hiding this comment

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

Maybe we need to add some tests

internal/locate/region_cache.go Show resolved Hide resolved
@hehechen
address comments
6683ffd 
Signed-off-by: hehechen <[email protected]>
@hehechen
add ut for GetMinSafeTS
427503c 
Signed-off-by: hehechen <[email protected]>
@hehechen
fix store type
0df1022 
Signed-off-by: hehechen <[email protected]>
@hehechen
Copy link
Contributor Author

/cc @sticnarf

@hehechen
Copy link
Contributor Author

/cc @disksing

sticnarf
sticnarf approved these changes Dec 19, 2022
View reviewed changes
Copy link
Collaborator

@sticnarf sticnarf left a comment

Choose a reason for hiding this comment

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

LGTM

@sticnarf
Copy link
Collaborator

@hehechen There are race and lint errors.

@hehechen hehechen requested a review from sticnarf December 19, 2022 07:45
@sticnarf
Copy link
Collaborator

There is still a lint error remaining.

You can ignore the integration test failure. It's just flaky.

@hehechen
fix CI
f4eab78 
Signed-off-by: hehechen <[email protected]>
@hehechen
Copy link
Contributor Author

There is still a lint error remaining.

You can ignore the integration test failure. It's just flaky.

Done. Checked in local machine by golangci-lint run.

@sticnarf sticnarf merged commit fe3536d into tikv:master Dec 19, 2022
@hehechen hehechen deleted the staleread branch December 19, 2022 09:31
@hehechen hehechen mentioned this pull request Dec 20, 2022
Merged
12 tasks
hawkingrei pushed a commit to hawkingrei/client-go that referenced this pull request Feb 24, 2023
@hehechen @hawkingrei
MinSafeTS support TiFlash (tikv#642)
497cb2c 
Signed-off-by: hehechen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JmPotato JmPotato JmPotato left review comments

@lhy1024 lhy1024 lhy1024 approved these changes

@sticnarf sticnarf Awaiting requested review from sticnarf

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hehechen @sticnarf @JmPotato @lhy1024