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

Nop ALTER SEQUENCE crashed Binlog Drainer due to SchemaVersion=0 #36276

Closed
kennytm opened this issue Jul 18, 2022 · 2 comments · Fixed by #36277
Closed

Nop ALTER SEQUENCE crashed Binlog Drainer due to SchemaVersion=0 #36276

kennytm opened this issue Jul 18, 2022 · 2 comments · Fixed by #36277
Assignees
Labels
affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects 5.4.x versions. affects-6.0 affects-6.1 component/binlog severity/moderate type/bug The issue is confirmed as a bug.

Comments

@kennytm
Copy link
Contributor

kennytm commented Jul 18, 2022

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

  1. Setup a TiDB → Binlog → (Anything) cluster

  2. On TiDB, execute the following SQL:

    CREATE SEQUENCE test.seq START WITH 200001;
    ALTER SEQUENCE test.seq START WITH 200001;

2. What did you expect to see? (Required)

The DDLs being successfully synchronized to drainer.

3. What did you see instead (Required)

Drainer crashed and cannot be restarted

[2022/07/18 12:36:45.700 +08:00] [ERROR] [main.go:69] ["start drainer server failed"] [error="version: 0 not found"] [errorVerbose="version: 0 not found\ngithub.com/pingcap/errors.NotFoundf\n\t/go/pkg/mod/github.com/pingcap/errors@v0.11.5-0.20211224045212-9687c2b0f87c/juju_adaptor.go:117\ngithub.com/pingcap/tidb-binlog/drainer.(*Schema).getSchemaTableAndDelete\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/tidb-binlog/drainer/schema.go:566\ngithub.com/pingcap/tidb-binlog/drainer.(*Syncer).run\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/tidb-binlog/drainer/syncer.go:468\ngithub.com/pingcap/tidb-binlog/drainer.(*Syncer).Start\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/tidb-binlog/drainer/syncer.go:151\ngithub.com/pingcap/tidb-binlog/drainer.(*Server).Start.func4\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/tidb-binlog/drainer/server.go:291\ngithub.com/pingcap/tidb-binlog/drainer.(*taskGroup).start.func1\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/tidb-binlog/drainer/util.go:79\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1571"] 

4. What is your TiDB version? (Required)

v5.4.0

@kennytm kennytm added type/bug The issue is confirmed as a bug. component/binlog affects-5.4 This bug affects 5.4.x versions. labels Jul 18, 2022
@kennytm
Copy link
Contributor Author

kennytm commented Jul 18, 2022

The root cause is this,

  1. When ALTER SEQUENCE is no-op, the shouldUpdateVer variable in onAlterSequence() is set to false:

    shouldUpdateVer := !reflect.DeepEqual(*tblInfo.Sequence, copySequenceInfo) || restart

  2. This leads to updateVersionAndTableInfo simply return ver=0

    tidb/ddl/table.go

    Lines 1337 to 1347 in 4cade24

    if shouldUpdateVer {
    ver, err = updateSchemaVersion(d, t, job)
    if err != nil {
    return 0, errors.Trace(err)
    }
    }
    if tblInfo.State == model.StatePublic {
    tblInfo.UpdateTS = t.StartTS
    }
    return ver, t.UpdateTable(job.SchemaID, tblInfo)

    tidb/ddl/sequence.go

    Lines 278 to 284 in 4cade24

    // Store the sequence info into kv.
    ver, err = updateVersionAndTableInfo(d, t, job, tblInfo, shouldUpdateVer)
    if err != nil {
    return ver, errors.Trace(err)
    }
    // Finish this job.
    job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tblInfo)

  3. Such ver=0 is used in (*Job).FinishTableJob() and eventually used to set its BinlogInfo.SchemaVersion

    tidb/parser/model/ddl.go

    Lines 410 to 414 in 4cade24

    func (job *Job) FinishTableJob(jobState JobState, schemaState SchemaState, ver int64, tblInfo *TableInfo) {
    job.State = jobState
    job.SchemaState = schemaState
    job.BinlogInfo.AddTableInfo(ver, tblInfo)
    }

    tidb/parser/model/ddl.go

    Lines 195 to 198 in 4cade24

    func (h *HistoryInfo) AddTableInfo(schemaVer int64, tblInfo *TableInfo) {
    h.SchemaVersion = schemaVer
    h.TableInfo = tblInfo
    }

  4. This value is propagated undisturbed to drainer (https://github.com/pingcap/tidb-binlog/blob/fa2543d767a29c98ad8f088e8878b294dab49a6a/drainer/syncer.go#L468), which caused the above NotFound error (https://github.com/pingcap/tidb-binlog/blob/fa2543d767a29c98ad8f088e8878b294dab49a6a/drainer/schema.go#L563-L566).

The DP devs felt that such SchemaVersion=0 binlogs breaks the monotonicity of the field, and thus the issue is filed here.

cc @lichunzhu @arenatlx

(internal reference: TICKET-353)

@niubell
Copy link
Contributor

niubell commented Jul 28, 2022

/remove-label affects-6.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects 5.4.x versions. affects-6.0 affects-6.1 component/binlog severity/moderate type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants