-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
http: add region-id when getting mvcc by handle #11436
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11436 +/- ##
===========================================
Coverage 81.2701% 81.2701%
===========================================
Files 425 425
Lines 92067 92067
===========================================
Hits 74823 74823
Misses 11860 11860
Partials 5384 5384 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
server/http_handler.go
Outdated
@@ -1293,6 +1310,9 @@ func (h mvccTxnHandler) handleMvccGetByKey(params map[string]string, decodeData | |||
colMap[col.ID] = &col.FieldType | |||
} | |||
|
|||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why add these lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest LGTM
server/http_handler.go
Outdated
Value *kvrpcpb.MvccGetByKeyResponse `json:"value"` | ||
Key string `json:"key"` | ||
Value *kvrpcpb.MvccGetByKeyResponse `json:"value"` | ||
RegionID uint64 `json:"region-id"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to use region_id
.
server/http_handler.go
Outdated
if err != nil { | ||
return nil, errors.Trace(err) | ||
} | ||
keyLocation, err := t.RegionCache.LocateKey(tikv.NewBackoffer(context.Background(), 500), encodedKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are many similar code like this. It is better to extract a function for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PTAL @jackysp
/run-all-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/run-all-tests |
@imtbkcat merge failed. |
/run-all-tests |
cherry pick to release-2.1 failed |
cherry pick to release-3.0 in PR #11557 |
It seems that, not for sure, we failed to cherry-pick this commit to release-2.1. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @imtbkcat PTAL. |
What problem does this PR solve?
return region id when getting mvcc info by handle.
What is changed and how it works?
Add
region-id
field, and get region info inhandleMvccGetByKey
.Check List
Tests
Code changes
Side effects
Related changes