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

[close #684] Fix batch get blocked by write #685

Merged
merged 10 commits into from
Dec 20, 2022

Conversation

shiyuhang0
Copy link
Collaborator

@shiyuhang0 shiyuhang0 commented Dec 15, 2022

Signed-off-by: shiyuhang [email protected]

What problem does this PR solve?

Issue Number: close #684

Problem Description: TBD

What is changed and how does it work?

Client-java's batchGet should retry to request TiKV with the ResolvedLocks context. So that the requests can ignore locks belonging to other transactions, because their commit_ts > read request's start_ts after pushing min_commit_ts in the first batchGet,

Just add ResolvedLocks filed in KVClient to cache the information and used it in the retry.

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has methods of interface change
  • Has persistent data change
  • No code

Check List for Tests

This PR has been tested by at least one of the following methods:

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

Side effects

  • Possible performance regression, WHY: TBD
  • Increased code complexity, WHY: TBD
  • Breaking backward compatibility, WHY: TBD
  • NO side effects

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note
  • NO related changes

@shiyuhang0 shiyuhang0 changed the title [close #684]fix batch get [close #684] Fix batch get blocked by write Dec 15, 2022
@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Base: 35.01% // Head: 36.86% // Increases project coverage by +1.84% 🎉

Coverage data is based on head (c938200) compared to base (1554ae5).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #685      +/-   ##
============================================
+ Coverage     35.01%   36.86%   +1.84%     
- Complexity     1446     1537      +91     
============================================
  Files           278      278              
  Lines         17382    17386       +4     
  Branches       1975     1976       +1     
============================================
+ Hits           6086     6409     +323     
+ Misses        10681    10322     -359     
- Partials        615      655      +40     
Impacted Files Coverage Δ
src/main/java/org/tikv/txn/KVClient.java 79.64% <100.00%> (+28.26%) ⬆️
src/main/java/io/grpc/internal/ClientCallImpl.java 56.82% <0.00%> (-0.25%) ⬇️
...ty/handler/codec/http2/Http2ConnectionHandler.java 51.58% <0.00%> (+0.48%) ⬆️
...ain/java/org/tikv/common/region/RegionManager.java 80.64% <0.00%> (+0.64%) ⬆️
...rc/main/java/io/grpc/netty/NettyClientHandler.java 56.89% <0.00%> (+0.64%) ⬆️
...java/org/tikv/common/region/RegionStoreClient.java 54.98% <0.00%> (+2.49%) ⬆️
...rc/main/java/org/tikv/common/util/ClientUtils.java 67.01% <0.00%> (+4.12%) ⬆️
src/main/java/org/tikv/common/region/TiStore.java 61.53% <0.00%> (+5.12%) ⬆️
...rc/main/java/org/tikv/common/meta/TiTimestamp.java 50.00% <0.00%> (+6.25%) ⬆️
...java/org/tikv/common/operation/KVErrorHandler.java 47.36% <0.00%> (+7.89%) ⬆️
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

shiyuhang0 and others added 7 commits December 20, 2022 17:35
Signed-off-by: shiyuhang <[email protected]>
Signed-off-by: shiyuhang <[email protected]>
Signed-off-by: shiyuhang <[email protected]>
Signed-off-by: shiyuhang <[email protected]>
Signed-off-by: shiyuhang <[email protected]>
Signed-off-by: zhangyangyu <[email protected]>
Signed-off-by: shiyuhang <[email protected]>
@ti-srebot
Copy link
Collaborator

@zhangyangyu, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. You are not a reviewer or committer or co-leader or leader.

@zhangyangyu zhangyangyu merged commit 1f096b5 into tikv:master Dec 20, 2022
ti-srebot pushed a commit to ti-srebot/client-java that referenced this pull request Dec 20, 2022
@ti-srebot
Copy link
Collaborator

cherry pick to release-3.3 in PR #691

shiyuhang0 added a commit to shiyuhang0/client-java that referenced this pull request Dec 30, 2022
iosmanthus pushed a commit to iosmanthus/client-java that referenced this pull request Dec 30, 2022
shiyuhang0 pushed a commit to shiyuhang0/client-java that referenced this pull request Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read is blocked by write
4 participants