pingcap / pingcap/tidb

Dead lock in local ingest add index

Open
#67,083 3 comments 0 reactions 1 assignee Claimed by @fzzf678 View on GitHub
component/ddl may-affects-7.1 may-affects-7.5 may-affects-8.1 may-affects-8.5 severity/major type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

Please answer these questions before submitting your issue. Thanks!

### 1. Minimal reproduce step (Required)

When adding multiple indexes in one DDL job, the local ingest path can deadlock between writer goroutines and `Flush()`. This can leave the subtask stuck for a very long time with no progress.

The subtask stays running but makes no forward progress. Goroutine dump shows:

* `(*litBackendCtx).Flush` stuck at `sync.RWMutex.Lock`
```
goroutine 1435 [sync.RWMutex.Lock, 84 minutes]:
sync.runtime_SemacquireRWMutex(0x4000f23ab8?, 0x10?, 0x4000f23c28?)
/usr/local/go/src/runtime/sema.go:105 +0x28
sync.(*RWMutex).Lock(0x4004365680?)
/usr/local/go/src/sync/rwmutex.go:155 +0xfc
github.com/pingcap/tidb/pkg/ddl/ingest.(*litBackendCtx).Flush(0x40044e6300, {0x6e78408, 0x40040641a0}, 0x0?)
/go/src/github.com/pingcap/tidb/pkg/ddl/ingest/backend.go:190 +0x19c
github.com/pingcap/tidb/pkg/ddl.(*indexIngestLocalWorker).HandleTask(0x4004468e40, {0x2, 0x402830c5a0, {0x0, 0x0}, 0x0, 0x40040641a0}, 0x404bab9f08)
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:830 +0x128
github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool.(*WorkerPool[...]).handleTaskWithRecover(0x6ed91a0, {0x6e4f088, 0x4004468e40}, {0x2, 0x402830c5a0, {0x0, 0x0}, 0x0, 0x40040641a0})
/go/src/github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool/workerpool.go:144 +0x190
github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool.(*WorkerPool[...]).runAWorker.func1()
/go/src/github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool/workerpool.go:160 +0x74
github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run.func1()
/go/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:157 +0x58
created by github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run in goroutine 1105
/go/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:155 +0x7c
```

* `WriteChunk` stuck at `sync.RWMutex.RLock`:
```
goroutine 1417 [sync.RWMutex.RLock, 84 minutes]:
sync.runtime_SemacquireRWMutexR(0x10?, 0xc0?, 0x1?)
/usr/local/go/src/runtime/sema.go:100 +0x28
sync.(*RWMutex).RLock(...)
/usr/local/go/src/sync/rwmutex.go:74
github.com/pingcap/tidb/pkg/ddl/ingest.(*writerContext).LockForWrite(0x4004620150)
/go/src/github.com/pingcap/tidb/pkg/ddl/ingest/engine.go:255 +0x6c
github.com/pingcap/tidb/pkg/ddl.writeChunk({0x6e78408, 0x40040641a0}, {0x4004367980, 0x2, 0x4001e37bd0?}, {0x40043678a0, 0x2, 0x2}, {0x6e65470, 0x4004387ac0}, ...)
/go/src/github.com/pingcap/tidb/pkg/ddl/index.go:2370 +0x16c
github.com/pingcap/tidb/pkg/ddl.(*indexIngestBaseWorker).WriteChunk(0x4004614900, 0x4001e37cc0)
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:938 +0x128
github.com/pingcap/tidb/pkg/ddl.(*indexIngestBaseWorker).HandleTask(0x4004614900, {0x3, 0x40043f9400, {0x0, 0x0}, 0x0, 0x40040641a0})
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:872 +0x6c
github.com/pingcap/tidb/pkg/ddl.(*indexIngestLocalWorker).HandleTask(0x4004614900, {0x3, 0x40043f9400, {0x0, 0x0}, 0x0, 0x40040641a0}, 0x4059824000)
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:821 +0xb0
github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool.(*WorkerPool[...]).handleTaskWithRecover(0x6ed91a0, {0x6e4f088, 0x4004614900}, {0x3, 0x40043f9400, {0x0, 0x0}, 0x0, 0x40040641a0})
/go/src/github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool/workerpool.go:144 +0x190
github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool.(*WorkerPool[...]).runAWorker.func1()
/go/src/github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool/workerpool.go:160 +0x74
github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run.func1()
/go/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:157 +0x58
created by github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run in goroutine 1105
/go/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:155 +0x7c
```

```
goroutine 1420 [sync.RWMutex.RLock, 84 minutes]:
sync.runtime_SemacquireRWMutexR(0x10?, 0xc0?, 0x1?)
/usr/local/go/src/runtime/sema.go:100 +0x28
sync.(*RWMutex).RLock(...)
/usr/local/go/src/sync/rwmutex.go:74
github.com/pingcap/tidb/pkg/ddl/ingest.(*writerContext).LockForWrite(0x40046206c0)
/go/src/github.com/pingcap/tidb/pkg/ddl/ingest/engine.go:255 +0x6c
github.com/pingcap/tidb/pkg/ddl.writeChunk({0x6e78408, 0x40040641a0}, {0x4004367b60, 0x2, 0x400169dbd0?}, {0x40043678a0, 0x2, 0x2}, {0x6e65470, 0x4004387ac0}, ...)
/go/src/github.com/pingcap/tidb/pkg/ddl/index.go:2370 +0x16c
github.com/pingcap/tidb/pkg/ddl.(*indexIngestBaseWorker).WriteChunk(0x4004614cc0, 0x400169dcc0)
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:938 +0x128
github.com/pingcap/tidb/pkg/ddl.(*indexIngestBaseWorker).HandleTask(0x4004614cc0, {0x3, 0x4000f37810, {0x0, 0x0}, 0x0, 0x40040641a0})
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:872 +0x6c
github.com/pingcap/tidb/pkg/ddl.(*indexIngestLocalWorker).HandleTask(0x4004614cc0, {0x3, 0x4000f37810, {0x0, 0x0}, 0x0, 0x40040641a0}, 0x4019ae0000)
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:821 +0xb0
github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool.(*WorkerPool[...]).handleTaskWithRecover(0x6ed91a0, {0x6e4f088, 0x4004614cc0}, {0x3, 0x4000f37810, {0x0, 0x0}, 0x0, 0x40040641a0})
/go/src/github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool/workerpool.go:144 +0x190
github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool.(*WorkerPool[...]).runAWorker.func1()
/go/src/github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool/workerpool.go:160 +0x74
github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run.func1()
/go/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:157 +0x58
created by github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run in goroutine 1105
/go/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:155 +0x7c
```

```
goroutine 1439 [sync.RWMutex.RLock, 84 minutes]:
sync.runtime_SemacquireRWMutexR(0x10?, 0xc0?, 0x1?)
/usr/local/go/src/runtime/sema.go:100 +0x28
sync.(*RWMutex).RLock(...)
/usr/local/go/src/sync/rwmutex.go:74
github.com/pingcap/tidb/pkg/ddl/ingest.(*writerContext).LockForWrite(0x400446f710)
/go/src/github.com/pingcap/tidb/pkg/ddl/ingest/engine.go:255 +0x6c
github.com/pingcap/tidb/pkg/ddl.writeChunk({0x6e78408, 0x40040641a0}, {0x4004064680, 0x2, 0x4001467bd0?}, {0x40043678a0, 0x2, 0x2}, {0x6e65470, 0x4004387ac0}, ...)
/go/src/github.com/pingcap/tidb/pkg/ddl/index.go:2370 +0x16c
github.com/pingcap/tidb/pkg/ddl.(*indexIngestBaseWorker).WriteChunk(0x4004469140, 0x4001467cc0)
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:938 +0x128
github.com/pingcap/tidb/pkg/ddl.(*indexIngestBaseWorker).HandleTask(0x4004469140, {0x2, 0x40707fcff0, {0x0, 0x0}, 0x0, 0x40040641a0})
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:872 +0x6c
github.com/pingcap/tidb/pkg/ddl.(*indexIngestLocalWorker).HandleTask(0x4004469140, {0x2, 0x40707fcff0, {0x0, 0x0}, 0x0, 0x40040641a0}, 0x4003c8e048)
/go/src/github.com/pingcap/tidb/pkg/ddl/backfilling_operators.go:821 +0xb0
github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool.(*WorkerPool[...]).handleTaskWithRecover(0x6ed91a0, {0x6e4f088, 0x4004469140}, {0x2, 0x40707fcff0, {0x0, 0x0}, 0x0, 0x40040641a0})
/go/src/github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool/workerpool.go:144 +0x190
github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool.(*WorkerPool[...]).runAWorker.func1()
/go/src/github.com/pingcap/tidb/pkg/resourcemanager/pool/workerpool/workerpool.go:160 +0x74
github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run.func1()
/go/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:157 +0x58
created by github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run in goroutine 1105
/go/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:155 +0x7c
```

`Flush()` and `writeChunk()` are waiting on the same set of per-index `flushLock`s from opposite directions.
* `writeChunk()` lock order
`writeChunk()` acquires all writers' `RLock`s in the order of the index slice:
https://github.com/pingcap/tidb/blob/16df0d15bb0f1b20402891ee5d95cf178253e1bb/pkg/ddl/index.go#L2671-L2674

* `Flush()` lock order
`Flush()` iterates bc.engines and acquires each engine's rw in `flushLock.Lock()`:
https://github.com/pingcap/tidb/blob/16df0d15bb0f1b20402891ee5d95cf178253e1bb/pkg/ddl/ingest/backend.go#L230-L239
However, `bc.engines` is a Go `map[int64]*engineInfo`, so the iteration order is not stable.

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

### 3. What did you see instead (Required)

### 4. What is your TiDB version? (Required)
v8.5.3

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.