global sort: step-scoped runtime slots can publish an incomplete unique index
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Bug Report
This is a correctness extension of #70408, but it uses the opposite resource transition and has a different terminal effect:
- #70408 reduces runtime slots after planning (`16 -> 4`), so existing merge outputs no longer fit the smaller ingest resource envelope and the job can fail.
- This report limits only the `read-index` step (`1` slot) and then lets `merge-sort` return to the task's larger `required_slots` value. Execution produces more merge outputs than the persisted plan validated, and the DDL can publish an incomplete unique index without returning an error.
### 1. Minimal reproduce step (Required)
#### A. Use the supported step-scoped runtime-slot control
Start a distributed Global Sort `ADD UNIQUE INDEX` task with `required_slots = 64`. While `read-index` is active, persist this documented DXF limit and let the executor reload the task metadata as described by the API documentation:
```shell
curl -X POST \
"http://{TiDBIP}:10080/dxf/task/{taskID}/max_runtime_slots?value=1&target_step=1"
```
For distributed add-index tasks, step `1` is `read-index`, step `2` is `merge-sort`, and step `3` is `ingest`. Therefore merge planning observes one runtime slot, but the limit no longer applies when merge execution starts; that step receives the original 64 slots.
The durable test below writes the same `MaxRuntimeSlots=1` and `TargetSteps=[read-index]` fields directly into task metadata before submission. This removes HTTP/reload timing from the test while preserving the production state transition.
#### B. Deterministic current-source cardinality witness
The following test runs in `pkg/ingestor/globalsort`, where `getTargetFileCount` is visible. All input files are assumed to overlap the same key range.
```go
func TestMergePlanRuntimeUpscale(t *testing.T) {
const (
inputFileCount = 31_500
nodeCount = 64
planningConcurrency = 1
executionConcurrency = 64
)
files := make([]string, inputFileCount)
groups, err := DivideMergeSortDataFiles(files, nodeCount, planningConcurrency)
require.NoError(t, err)
require.Len(t, groups, 128)
targetFiles := 0
for _, group := range groups {
targetFiles += getTargetFileCount(len(group), executionConcurrency)
}
require.Equal(t, 8_192, targetFiles)
ingestOverlapTarget := int(simplesst.GetAdjustedMergeSortOverlapThreshold(executionConcurrency))
require.Equal(t, 4_000, ingestOverlapTarget)
require.Greater(t, targetFiles, ingestOverlapTarget)
}
```
`DivideMergeSortDataFiles` accepts this plan at concurrency 1 and creates 128 groups. Executing those groups at concurrency 64 targets 8,192 output files. This exceeds both the 4,000-file planning target and the 8,000 aggregate reader limit used by `NewMergePropIter`.
The bounded merge iterator also has a direct ordering witness. Three overlapping streams `[1,100]`, `[2,101]`, and `[3,102]` under reader limit 2 should produce:
```text
1, 2, 3, 100, 101, 102
```
Current code produces:
```text
1, 2, 100, 101, 3, 102
```
The iterator delays opening the third overlapping reader, so its first key can be smaller than keys already emitted from the first two readers.
#### C. Real-TiKV durable consequence
I lifted the same transition through the distributed DDL state machine, real merge workers, SST writing/ingest, and a real TiKV store. To keep the test small, the test uses 32 external files, execution concurrency 4, and a merge-property reader limit of 2. These hooks compress the file-count and reader-capacity constants; they do not change the index rows, DDL terminal state, merge implementation, or TiKV ingest path.
Test shape:
```sql
CREATE TABLE t (
id INT PRIMARY KEY CLUSTERED,
u INT NOT NULL
);
-- Insert 40,000 rows with 40,000 distinct u values.
ALTER TABLE t ADD UNIQUE INDEX idx_u(u);
```
The task metadata uses `required_slots=4`, `MaxRuntimeSlots=1`, and `TargetSteps=[read-index]`. The observed step resources and merge shape are:
```text
read-index: CPU=1
merge-sort: CPU=4
inputFiles=32 mergeConcurrency=4 outputGroups=4
propStats=4 propWeight=4 propLimit=2
ddlErr=
DDL job state=done schemaState=public
```
After the DDL reports success:
```sql
SELECT COUNT(*) FROM t FORCE INDEX(PRIMARY); -- 40000
SELECT COUNT(*) FROM t FORCE INDEX(idx_u); -- 34960
ADMIN CHECK TABLE t; -- ERROR 8223
-- u=1000001 already exists in the base table but is absent from idx_u.
INSERT INTO t VALUES (40002, 1000001); -- succeeds
SELECT COUNT(*) FROM t FORCE INDEX(PRIMARY)
WHERE u = 1000001; -- 2
```
There is no concurrent DML during `ADD INDEX`. A stable `1 -> 1` control on the same 40,000 rows produces `40000/40000`, passes `ADMIN CHECK TABLE`, and rejects the duplicate. A counterfactual that persists the planning concurrency in merge-subtask metadata and executes at `min(planned, current)` is also `40000/40000` and rejects the duplicate while the task still has four required slots.
The real-TiKV consequence test is scale-compressed. The natural 31,500-file end-to-end run has not yet been completed. The exact current-source calculation proves the violation once such a 31,500-file overlapping group exists, but it does not establish how frequently a production job creates that file shape.
#### D. Source-level cause
`generateMergeSortPlan` uses `task.GetRuntimeSlots()` to call `DivideMergeSortDataFiles`, but `BackfillSubTaskMeta` stores only the selected file group and element IDs. It does not store the concurrency used to validate the plan.
Later, `mergeSortExecutor.RunSubtask` passes the current `StepResource` CPU capacity to both `NewMergeOperator` and `MergeOverlappingFiles`. `MergeOverlappingFiles` splits each persisted file group again using that newer concurrency. A step-scoped limit can therefore make planning use one resource generation and execution expand the same plan under another.
The merge-property iterator assumes upstream overlap is within its bounded-reader capacity. When execution creates more mutually overlapping outputs than that capacity, property order is no longer globally sorted. In the real-TiKV lift, the resulting range partitioning omits index KVs while every DDL stage still returns success.
### 2. What did you expect to see? (Required)
Changing the effective resource between DDL steps must not invalidate a persisted merge plan. In particular:
- merge execution must not emit more overlapping files than the plan and downstream readers can preserve;
- if the resource transition is unsafe, the task must regenerate/revalidate the plan or fail before ingest;
- `ADD UNIQUE INDEX` must not reach `public/done` unless every base row has exactly one index entry and future uniqueness enforcement is complete.
Persisting the plan-time concurrency and preventing execution from upscaling past it closes this specific `1 -> 64` branch. The `16 -> 4` downscale described in #70408 still needs its own compatibility validation or plan regeneration; pinning concurrency alone is not proposed as the complete fix for both directions.
### 3. What did you see instead (Required)
The merge plan is validated under one runtime-slot generation and expanded under another. At production constants, 31,500 input files become 8,192 target files after the `1 -> 64` transition. In the scale-compressed real-TiKV lift, the DDL returns success and publishes the unique index with 5,040 missing entries. `ADMIN CHECK TABLE` detects the corruption only afterward, and the missing index entries allow duplicate values to be inserted.
### 4. What is your TiDB version? (Required)
```text
Git Commit Hash: 8b62b76214af40eed6aeac045948e2d53adbb7e3
Git Branch: master
Nearest tag: v9.0.0-beta.2.pre-2088-g8b62b76214a
```
This commit already contains the exact target-count changes from #70352.
Contributor guide
Research direction
Start in pkg/ingestor/globalsort and trace generateMergeSortPlan, BackfillSubTaskMeta, mergeSortExecutor.RunSubtask, and MergeOverlappingFiles. Run the provided cardinality and bounded-reader witnesses first, then inspect the durable consequence test. Done means unsafe resource changes are revalidated or rejected before ingest, with the unique-index integrity checks passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100