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

*: fix panic that upgrade from old version TiDB. #7136

Merged
merged 6 commits into from
Jul 24, 2018
Merged

*: fix panic that upgrade from old version TiDB. #7136

merged 6 commits into from
Jul 24, 2018

Conversation

winkyao
Copy link
Contributor

@winkyao winkyao commented Jul 24, 2018 •
edited
Loading

What have you changed? (mandatory)

Upgrade tidb from 2.0.5 to master got panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0xc55fa9]


goroutine 1 [running]:
github.com/pingcap/tidb/table/tables.(*index).truncateIndexValuesIfNeeded(0xc420bc54a0, 0xc420e4b980, 0x1, 0x1, 0x0, 0xc420d0ad40, 0x410139)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/table/tables/index.go:135 +0x79
github.com/pingcap/tidb/table/tables.(*index).GenIndexKey(0xc420bc54a0, 0xc42144ee60, 0xc420e4b980, 0x1, 0x1, 0x0, 0x0, 0x0, 0x0, 0x1, ...)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/table/tables/index.go:174 +0x12f
github.com/pingcap/tidb/executor.(*batchChecker).getKeysNeedCheck(0xc420689a80, 0x1444400, 0xc42083d500, 0x1444ac0, 0xc42057cea0, 0xc4201716a0, 0x1, 0x1, 0xc4201ea0d0, 0xc420d0b158, ...)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/executor/batch_checker.go:136 +0x3ea
github.com/pingcap/tidb/executor.(*batchChecker).batchGetInsertKeys(0xc420689a80, 0x1444400, 0xc42083d500, 0x1444ac0, 0xc42057cea0, 0xc4201716a0, 0x1, 0x1, 0x2, 0xc)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/executor/batch_checker.go:170 +0x9e
github.com/pingcap/tidb/executor.(*InsertExec).batchUpdateDupRows(0xc420e4b900, 0xc4201716a0, 0x1, 0x1, 0xc42083d510, 0x0)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/executor/insert.go:119 +0x9a
github.com/pingcap/tidb/executor.(*InsertExec).exec(0xc420e4b900, 0xc4201716a0, 0x1, 0x1, 0x0, 0x0)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/executor/insert.go:75 +0x28e
github.com/pingcap/tidb/executor.(*InsertExec).(github.com/pingcap/tidb/executor.exec)-fm(0xc4201716a0, 0x1, 0x1, 0x3, 0xc420dad0b0)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/executor/insert.go:185 +0x48
github.com/pingcap/tidb/executor.(*InsertValues).insertRows(0xc420689a20, 0xc420bc5460, 0x3, 0x3, 0xc420d0b5e8, 0x3, 0x3)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/executor/insert_common.go:198 +0x30d
github.com/pingcap/tidb/executor.(*InsertExec).Next(0xc420e4b900, 0x7f5cc9d3e490, 0xc42003c058, 0xc420171680, 0x14266a0, 0xc420e4b900)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/executor/insert.go:187 +0xe4
github.com/pingcap/tidb/executor.(*ExecStmt).handleNoDelayExecutor(0xc420569580, 0x7f5cc9d3e490, 0xc42003c058, 0x1444400, 0xc42083d500, 0x14266a0, 0xc420e4b900, 0x7f5cc9c6e2d8, 0xc42083d500, 0x0, ...)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/executor/adapter.go:267 +0x17a
github.com/pingcap/tidb/executor.(*ExecStmt).Exec(0xc420569580, 0x7f5cc9d3e490, 0xc42003c058, 0x0, 0x0, 0x0, 0x0)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/executor/adapter.go:227 +0x4e2
github.com/pingcap/tidb/session.runStmt(0x7f5cc9d3e490, 0xc42003c058, 0x1444400, 0xc42083d500, 0x14233e0, 0xc420569580, 0x0, 0x0, 0x0, 0x0)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/session/tidb.go:154 +0x211
github.com/pingcap/tidb/session.(*session).executeStatement(0xc42083d500, 0x7f5cc9d3e490, 0xc42003c058, 0x0, 0x14212a0, 0xc42144edc0, 0x14233e0, 0xc420569580, 0x0, 0x0, ...)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/session/session.go:739 +0x1e7
github.com/pingcap/tidb/session.(*session).execute(0xc42083d500, 0x7f5cc9d3e490, 0xc42003c058, 0xc420386870, 0x82, 0x0, 0x0, 0x0, 0x0, 0x0)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/session/session.go:807 +0x7c6
github.com/pingcap/tidb/session.(*session).Execute(0xc42083d500, 0x7f5cc9d3e490, 0xc42003c058, 0xc420386870, 0x82, 0xc42066a900, 0xc420bfc400, 0x82, 0x400, 0xc420386870)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/session/session.go:756 +0x69
github.com/pingcap/tidb/session.mustExecute(0x1446ac0, 0xc42083d500, 0xc420386870, 0x82)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/session/bootstrap.go:751 +0x91
github.com/pingcap/tidb/session.updateBootstrapVer(0x1446ac0, 0xc42083d500)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/session/bootstrap.go:657 +0x11b
github.com/pingcap/tidb/session.upgrade(0x1446ac0, 0xc42083d500)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/session/bootstrap.go:410 +0x18e
github.com/pingcap/tidb/session.runInBootstrapSession(0x142bec0, 0xc4203c6000, 0x136ad70)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/session/session.go:1135 +0xe3
github.com/pingcap/tidb/session.BootstrapSession(0x142bec0, 0xc4203c6000, 0x0, 0x0, 0x0)
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/session/session.go:1084 +0x313
main.createStoreAndDomain()
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/tidb-server/main.go:164 +0x152
main.main()
        /home/jenkins/workspace/build_tidb_master/go/src/github.com/pingcap/tidb/tidb-server/main.go:142 +0x6b

The panic is cause by #7109,

tidb/table/tables/index.go

Line 135 in 48a42fe

if ic.Tp.Charset == charset.CharsetUTF8 || ic.Tp.Charset == charset.CharsetUTF8MB4 {
, that PR modify the IndexInfo, which will cause a compatibility problem. I change the implementation in this PR to fix it.

What is the type of the changes? (mandatory)

  • Bug fix (non-breaking change which fixes an issue)

How has this PR been tested? (mandatory)

exist tests.

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

NO

Does this PR affect tidb-ansible update? (mandatory)

NO

Does this PR need to be added to the release notes? (mandatory)

NO

Refer to a related PR or issue link (optional)

#7109 #7126

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

@winkyao winkyao added type/bugfix This PR fixes a bug. priority/release-blocker This issue blocks a release. Please solve it ASAP. labels Jul 24, 2018
zimulala
zimulala reviewed Jul 24, 2018
View reviewed changes
table/tables/index.go Outdated
index := &index{
idxInfo: indexInfo,
prefix: tablecodec.EncodeTableIndexPrefix(id, indexInfo.ID),
tblInfo: tblInfo,
// The prefix can't Encode from tblInfo.ID, because table partition may change the id to partition id.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Encode/encode

@winkyao
Copy link
Contributor Author

winkyao commented Jul 24, 2018

@zimulala PTAL

zimulala
zimulala reviewed Jul 24, 2018
View reviewed changes
Copy link
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

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

LGTM
We need to cherry pick it to release-2.0.

@zimulala zimulala added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 24, 2018
@jackysp
Copy link
Member

jackysp commented Jul 24, 2018 •
edited
Loading

How to know it could work? Please add a manual test result.

@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added component/DDL-need-LGT3 status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 24, 2018
crazycs520
crazycs520 reviewed Jul 24, 2018
View reviewed changes
Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@crazycs520 crazycs520 added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jul 24, 2018
@winkyao winkyao merged commit 5f7abc8 into pingcap:master Jul 24, 2018
@winkyao winkyao deleted the fix_panic branch July 24, 2018 10:59
@winkyao winkyao mentioned this pull request Jul 24, 2018
Merged
winkyao added a commit that referenced this pull request Jul 24, 2018
@winkyao
*: fix panic that upgrade from old version TiDB. (#7136) (#7144)
8bac140
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crazycs520 crazycs520 crazycs520 left review comments

@tiancaiamao tiancaiamao tiancaiamao approved these changes

@zimulala zimulala zimulala approved these changes

Assignees
No one assigned
Labels
priority/release-blocker This issue blocks a release. Please solve it ASAP. status/LGT3 The PR has already had 3 LGTM. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@winkyao @jackysp @tiancaiamao @zimulala @crazycs520