-
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
ddl, util/codec: fix add index OOM and prevent panic in logging (#29925) #30126
Conversation
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
/run-all-tests |
@tangenta you're already a collaborator in bot's repo. |
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
This pull request is closed because it's related version has closed automatic cherry-picking. You can find more details at: |
cherry-pick #29925 to release-5.2
You can switch your code base to this Pull Request by using git-extras:
# In tidb repo: git pr https://github.com/pingcap/tidb/pull/30126
After apply modifications, you can push your change to this PR via:
What problem does this PR solve?
Issue Number: close #27687
Reproducible step:
Problem Summary:
This problem may occur in the following conditions:
There is a bug in the current implementation of
NewCommonHandle()
:tidb/kv/key.go
Lines 237 to 262 in 37bfd57
If the argument bytes have a prefix like
0x8203
,CutOne()
cuts nothing so that theremain
is unchanged in the loop. This dead loop may cause TiDB OOM quickly because it endlesslyappend()
colEndOffsets
.What is changed and how it works?
peek()
in the decoding process does not handle the invalid key. It is possible to run into a dead loop and makes TiDB OOM.tryDecodeHandleToString()
is only used for logging, it should be wrapped withrecover()
so that the critical operation is not interrupted.Check List
Tests
Side effects
Documentation
Release note