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

Fix ResolveLock not called in Scan API #2089

Merged
merged 2 commits into from
Jul 30, 2021

Conversation

birdstorm
Copy link
Contributor

ResolveLock was not called in Scan API, it would lead to

  • Sometimes the table/database info was not retrieved from TiKV, e.g., table or view not found
  • Scan API may not get all KV pairs if some of them are locked by another transaction.

fix resolve lock not called in Scan API

Signed-off-by: birdstorm <[email protected]>
@birdstorm birdstorm added needs-cherry-pick-2.3 needs-cherry-pick-release-2.4 PR which needs to be cherry-picked to release-2.4 labels Jul 23, 2021
@birdstorm
Copy link
Contributor Author

/run-all-tests tidb=v5.0.3 tikv=v5.0.3 pd=v5.0.3

Signed-off-by: birdstorm <[email protected]>
@birdstorm
Copy link
Contributor Author

/run-all-tests tidb=v5.0.3 tikv=v5.0.3 pd=v5.0.3

Copy link
Collaborator

@marsishandsome marsishandsome left a comment

Choose a reason for hiding this comment

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

this PR should port to client-java ?

@@ -115,7 +118,7 @@ public static ByteString bytesGet(ByteString key, Snapshot snapshot) {
List<Pair<ByteString, ByteString>> fields = new ArrayList<>();
while (iterator.hasNext()) {
Kvrpcpb.KvPair kv = iterator.next();
if (kv == null || kv.getKey() == null) {
if (kv == null || kv.getKey().isEmpty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (kv == null || kv.getKey().isEmpty()) {
if (kv == null || kv.getKey() == null || kv.getKey().isEmpty()) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marsishandsome key is non-null

Copy link
Collaborator

@marsishandsome marsishandsome left a comment

Choose a reason for hiding this comment

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

LGTM

@birdstorm
Copy link
Contributor Author

/merge

@birdstorm birdstorm merged commit ddbf7d3 into pingcap:master Jul 30, 2021
@birdstorm birdstorm deleted the fix-resolve-lock-in-scan branch July 30, 2021 07:11
@ti-srebot
Copy link
Collaborator

cherry pick to release-2.3 in PR #2090

@ti-srebot
Copy link
Collaborator

cherry pick to release-2.4 in PR #2091

birdstorm pushed a commit that referenced this pull request Aug 2, 2021
fix resolve lock not called in Scan API

Signed-off-by: birdstorm <[email protected]>
birdstorm added a commit that referenced this pull request Sep 6, 2021
* fix v2.4 scan bug

fix resolve lock not called in Scan API

Signed-off-by: birdstorm <[email protected]>

* remove backoff

Signed-off-by: birdstorm <[email protected]>

Co-authored-by: birdstorm <[email protected]>
wfxxh pushed a commit to wanfangdata/tispark that referenced this pull request Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-2.4 PR which needs to be cherry-picked to release-2.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants