matrixorigin / matrixorigin/matrixone

[Bug]: REPLACE/UPDATE can commit duplicate primary-key rows on composite-PK table with non-unique secondary indexes

Open
#24,975 0 comments 0 reactions 1 assignee Claimed by @ck89119 View on GitHub
ai-medium deferred kind/bug level/hard-to-repro
Dominant language
Go
Stars
1.9k
Forks
311
Avg merge
1d 3h
Merged PRs (30d)
768

Description

### Is there an existing issue for the same bug?

- [x] I have checked the existing issues.

### Branch Name

Observed on a deployed build at commit `2b7018a66`. The relevant code paths are also present on `main` / `3.0-dev` / `4.0-dev` (verified byte-identical for the serial/cpkey path).

### Commit ID

`2b7018a66`

### Other Environment Information

- Deployment: multi-CN cluster (3 CN), shared-storage.
- Table `datasync.tmp` (a datasync file-sync table; the genesis `UPDATE` ran on the sibling `datasync.file`). Schema:

```sql
CREATE TABLE datasync.tmp (
task_id uuid NOT NULL,
source_file_path varchar(1000) NOT NULL,
sink_file_path varchar(1000) NOT NULL,
hash varchar(128) NOT NULL,
dup_file_path varchar(1000) NOT NULL,
governed_results json NOT NULL,
last_modified_time timestamp NULL DEFAULT NULL,
last_begin_time timestamp NULL DEFAULT NULL,
last_end_time timestamp NULL DEFAULT NULL,
PRIMARY KEY (task_id, source_file_path, sink_file_path), -- composite -> hidden __mo_cpkey_col = serial(task_id, source_file_path, sink_file_path)
KEY source_file_path (source_file_path),
KEY sink_file_path (sink_file_path),
KEY hash (hash),
KEY dup_file_path (dup_file_path),
KEY last_modified_time (last_modified_time),
KEY last_begin_time (last_begin_time) -- 6 NON-UNIQUE secondary indexes
);
```
~3.77M rows.
- Workload: high-concurrency single-row `INSERT` / `UPDATE` / `REPLACE` from a sync job; the time window of the incident also showed many statements failing with `connection reset by peer` between CNs and one CN going down.

### Actual Behavior

A table **with a primary key** accumulated rows that violate PK uniqueness. Two distinct corruption signatures coexist (verified by exact-equality queries, not the racy `GROUP BY`):

**Signature A — `__mo_cpkey_col` off-by-one row shift (the seed, exactly 15 rows / 3.77M).**

For 15 rows, the hidden composite key does **not** match the row's own primary-key columns:
`serial(task_id, source_file_path, sink_file_path) != __mo_cpkey_col`.

The misaligned `__mo_cpkey_col` is a **complete, valid cpkey** — but it is the cpkey of the **next row** (`__mo_rowid` offset + 1). I.e. the `__mo_cpkey_col` column is **shifted by one row** relative to the PK data columns:

```
stored __mo_cpkey_col[i] == serial(primary key of row i+1)
```

Verified on the live table: of the 15 misaligned rows, **13** have their `__mo_cpkey_col` equal to `serial(pk)` of the immediately-following `__mo_rowid` (tail i → tail i+1); the other 2 chain to other already-misaligned rows.

Concrete example (decoded `__mo_cpkey_col`):

```
__mo_rowid ...-0-3-6166 : columns = .../10月/5000397771.pdf
__mo_cpkey_col decodes to serial(task_id, .../3月/5000311902.pdf, .../3月/5000311902.pdf)
__mo_rowid ...-0-3-6167 : columns = .../3月/5000311902.pdf <-- the next row; row 6166 carries 6167's key

row 6166: source_file_path/sink_file_path columns = 5000397771 (self-consistent)
__mo_cpkey_col = key of 5000311902 (the NEXT row)
```

Note: row i's `__mo_cpkey_col` is **not byte-equal** to what is *currently stored* in row i+1's `__mo_cpkey_col` (0/15), because row i+1's own `__mo_cpkey_col` is itself shifted/corrupted. It equals the *correct* `serial(pk)` of row i+1. This is a textbook off-by-one of the `__mo_cpkey_col` vector against the data vectors during the distributed write.

Detection:
```sql
SELECT `__mo_rowid`, source_file_path, sink_file_path, HEX(`__mo_cpkey_col`)
FROM datasync.tmp
WHERE HEX(serial(task_id, source_file_path, sink_file_path)) <> HEX(`__mo_cpkey_col`);
-- 15 rows
```

**Signature B — exact clones (the amplification, up to 1,048,576 copies of one key).**
Same PK columns **and** same (correct) `__mo_cpkey_col`, the row duplicated `2^k` times. Distribution of duplicate counts: all powers of two (2, 8, 16, 32, 1024, 1048576). The largest group: one key with **1,048,576 byte-identical rows** (distinct `__mo_rowid`, identical `__mo_cpkey_col`, identical non-PK columns).

### Expected Behavior

On a primary-key table, it must be **impossible** to commit two rows with the same primary key — regardless of node failures / connection resets during distributed execution. Any duplicate (2 or 1,048,576) is an equal correctness violation.

### Steps to Reproduce

> Not deterministically reproducible. The amplification (Signature B) is fully understood and demonstrable; the seed (Signature A) is a rare runtime event that we could **not** trigger with fault injection (see below).

**Amplification (Signature B) — root mechanism, confirmed via `EXPLAIN ANALYZE` of the genesis statement:**

The genesis statement was a single-row, full-PK `UPDATE` (changing only non-PK columns):

```sql
UPDATE t SET hash = '...', dup_file_path = '', governed_results = '{...}', ...
WHERE c1 = '...' AND c2 = '...' AND c3 = '...'; -- 29.9s, "Success"
```

Its plan fans out via the **index-maintenance INNER joins**. The `EXPLAIN ANALYZE` row ladder:

```
Table Scan (16 dup rows already present for this key)
--INNER JOIN (idx)--> 256 (16 x 16)
--INNER JOIN (idx)--> 4096
--INNER JOIN (idx)--> 65536
--INNER JOIN (idx)--> 1048576 = 16^5
Multi Update: inserts 1,048,576 rows
```

Code: `pkg/sql/plan/bind_update.go` — the "join index tables to get old RowID" step uses `plan.Node_INNER` for **non-unique** secondary indexes:

```go
joinType := plan.Node_LEFT
if !idxDef.Unique && !isSpatialIndexDef(idxDef) {
joinType = plan.Node_INNER
}
```

Join condition is `serial_full(old_idxcol, old_cpkey) = idx.__mo_index_idx_col`, which is 1:1 when the key is unique, but becomes `D x D` per index (chained → `D^(numIndexes+1)`) once the main table **and** index tables already hold `D` duplicates. So once a key is duplicated, the next `UPDATE`/`REPLACE` touching it multiplies it.

**Seed (Signature A) — runtime, NOT a plan-binding bug:**

`bind_update.go`'s final projection reads every main-table column (including `__mo_cpkey_col`) from the **same** source node at its own colIdx, so cpkey and data are statically aligned — a plan-level swap is impossible. Therefore the misalignment must be introduced at **runtime** (a shuffle/join/vector operation reordering the `__mo_cpkey_col` vector independently of the data vectors), under distributed execution disrupted by node instability.

**Fault-injection attempts that did NOT reproduce (local 2-CN cluster):**
- Clean distributed bulk `UPDATE` (400K rows, INNER-join plan confirmed): 0 mismatch.
- `cn2` 40% packet loss + 120ms delay + 15 concurrent distributed `UPDATE`s: 0 mismatch.
- `cn2` restart mid-flight ×N, `cn1` OOM mid-insert: clean rollback, 0 mismatch.

In all injected faults MO maintained PK consistency (clean rollback). The production seed is rarer/more specific than these.

### Additional information

**Why this is two layers, and which is root:**
- `INSERT` correctly rejects same-cpkey duplicates with `ERROR 1062`. So PK dedup works **when cpkey is consistent**. Duplicates can only slip through if the cpkey is wrong/misaligned (Signature A) → dedup compares the wrong key and misses the conflict → duplicate committed → then amplified by Signature B.
- **Root = Signature A** (a distributed write must never lose PK uniqueness, even under connection reset). Fixing only Signature B (the INNER-join fan-out) limits blast radius but does not address the root.

**Suggested fixes:**
1. **(root)** Guarantee PK uniqueness of distributed writes under node failure / connection reset — make cross-CN conflict detection fault-tolerant, or verify cpkey/PK consistency before commit and abort on mismatch. Requires first locating where `__mo_cpkey_col` gets row-misaligned in the distributed write/dedup path (a cpkey-consistency assertion `serial(pk) == __mo_cpkey_col` at the main-table write boundary detects it with zero false positives on clean data; recommended for catching the seed on a canary under real workload).
2. **(defense-in-depth)** Make the `bind_update.go` index-maintenance join idempotent for already-duplicated keys (1:1 by rowid), so any seed cannot be exponentially amplified.

**Detection query** (find misaligned rows on any affected table):

```sql
SELECT * FROM t WHERE HEX(serial(c1, c2, c3)) <> HEX(`__mo_cpkey_col`);
```

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.