yujun777 commented on issue #68679:
URL: https://github.com/apache/doris/issues/68679#issuecomment-6142250468
## A systematic pass over every point between the two halves
Added after #68662 reached master. The three numbered paths in the body are
a subset of the list below; this comment enumerates all of them with the code
path for each, and separates **confirmed** (read on master) from **pending**
(needs a reproduction).
### Where the two durable facts separate
- The **rows and the table-stream offsets** become durable together at the
inner insert's commit: `InsertIntoTableCommand` hangs the offsets on the
transaction, and `DatabaseTransactionMgr.updateCatalogAfterCommitted` applies
them on commit and replays them.
- The **publication** is a second, separate catalog action:
`InsertOverwriteUtil.replacePartition` → `Env.replaceTempPartition` /
`ReplacePartitionOperationLog`, driven from `InsertOverwriteTableCommand`
(explicit partitions) or `InsertOverwriteManager.replacePartitionsOfTaskGroup`
(`PARTITION(*)`).
- In between, the system holds **temp partitions that are durable but
unpublished** — `InsertOverwriteManager` is itself persisted (image plus its
own `InsertOverwriteLog` ADD/DROP/CANCEL journal records) — and **offsets that
are already advanced**.
### Local (OLAP) target
1. **FE crash between the halves** — confirmed. Nothing publishes the
committed rows: after a restart `InsertOverwriteManager.runAfterCatalogReady`
only rolls back tasks that are *marked cancel*, so the task and its temp
partitions survive, unpublished, while the offsets stay advanced. A re-run
reads from the advanced position and that range is silently missing.
2. **Master switch** — confirmed. `Env.transferToMaster` → `allTaskFail()` →
`taskFail` → `rollback` **drops** every in-flight task's temp partitions. The
committed rows are gone entirely and the offsets stay advanced.
3. **The swap itself fails** — confirmed. `replacePartitionsOfTaskGroup`
catches, logs, and rethrows; `taskGroupFail`/`taskFail` → `rollback` drops the
temp partitions. The error does reach the client, but nothing moves the offsets
back, so a re-run still skips the range.
4. **Cancellation and the error response** — fixed by #68662: before the
insert, after an insert that committed nothing, while the swap waits for the
target's write lock, and the `insert_visible_timeout_return_mode=error`
publication timeout, plus a target dropped under the swap.
5. **Crash inside `finishTaskGroup`** — checked, and the outcome is *not*
what I expected when writing this list. The replacement has already run, so the
rows are published and correct; only the per-task bookkeeping
(`logInsertOverwrite(ADD)` then `taskSuccess`, which logs DROP) is half done,
leaving some tasks registered. Those are never cleaned (`runAfterCatalogReady`
handles only cancelled tasks), but they are also **harmless**: after the swap
the temp names no longer exist — the replacement runs with
`PROPERTIES_USE_TEMP_PARTITION_NAME=false`, so the new data takes the
*original* partition names — and `InsertOverwriteUtil.dropPartitions` looks the
name up with `getPartition(name, true)`, which reads only `tempPartitions` and
`continue`s when it is absent. So a later `taskFail`/`allTaskFail` over such a
task is a no-op that removes it, and it cannot reach the live partition. What
is left is a stale entry in `tasks`, not data loss.
One narrow residual, **pending a reproduction**: the temp names are
generated as `iot_temp_<threadId>_<partitionName>` with no statement identity
in them, and the swap frees that name again. If a crash leaves a task
registered and a later overwrite on the same table happens to run on a reused
thread id, it recreates the same temp name — and a master switch would then run
`allTaskFail` over the *stale* task and drop that live statement's temp
partitions, after its rows were committed. Recorded because removing the stale
entry is the smaller half of the cleanup.
### Remote target (`RemoteOlapTable`, `RemoteOlapInsertExecutor`,
`RemoteInsertOverwriteManager`)
6. **A lost commit reply** — confirmed as a reachable state, pending as a
reproduction. `masterCallWithRetry` can exhaust its retries *after* the owning
FE committed; this FE then cannot tell a committed transaction from a failed
one, and `onFail`'s abort cannot take a committed remote transaction back — so
the overwrite rolls back temp partitions holding durable rows. Note that the
remote commit request does **not** carry stream updates, so the offset half of
this needs its own reproduction before it is claimed.
7. **A cancellation arriving while the owner waits for the table write
lock** in `replacePartitionsImpl` — confirmed as a protocol gap: the
replacement RPC carries no cancellation state, so an overwrite whose insert
committed nothing can still publish an empty result and report success.
8. **An owner-side crash or master switch** — the same shape as 1 and 2, but
on the frontend that owns the swap.
### Cloud
9. **An uncertain commit** — `KV_TXN_COMMIT_ERR` arrives after the finite
`KV_TXN_MAYBE_COMMITTED` retries, with the meta service having possibly applied
the commit; `commitAndPublishTransactionWithRetry` then throws and the
overwrite treats it as a failure, dropping temp partitions whose rows *and
whose stream offsets* the meta service may already hold.
10. Cloud has no FE publication phase
(`CloudGlobalTransactionMgr.commitAndPublishTransaction` returns true after the
commit), so the publication-timeout shape does not exist there — which is why
the regression case added in #68662 is guarded by `!isCloudMode()`.
### External / plugin targets
11. For a non-OLAP target the FE creates no temp partitions at all ("Do not
create temp partition on FE", `partitionNames` empty) and the two phases belong
to the connector. Whether the same commit/publish split exists there is **not
established** here.
### The uncertain-commit half is not specific to INSERT OVERWRITE
The cloud half is a cloud-transaction-semantics question rather than an
overwrite one, and `INSERT INTO` reaches the same state — see the thread on
`OlapInsertExecutor`. What differs is the direction, and it is worth stating
explicitly because it is why only the overwrite is silent:
- `INSERT INTO` commits its rows into the target's own partitions and
attaches the stream offsets to **the same transaction**, so an uncertain commit
is all-of-it or none-of-it. The symptom is a **false failure**: the client is
told the load failed while the rows and the offsets are durable, so a retry
duplicates. There is no second phase that can drop what was committed.
- The loss in `INSERT OVERWRITE` comes from the combination: rows committed
into **temporary** partitions, which the failure path drops, with the offsets
advanced by that same transaction.
That places the cloud part in the transaction layer (the owner has to be
able to answer whether the commit happened), which is what #68662's scope note
already concludes; the overwrite-specific part is what the other ten paths have
in common.
### Related
- #65265 (binlog umbrella), #65418 (incremental-computation tracker) — the
IVM side of this.
- #68662 — fixes the local cancellation and error-response halves.
- #68390 — the IVM partition refresh journals a rebuild requirement before
it reads, which covers the failure and master-switch halves for a refresh; the
refresh executes through this same two-phase command. Its local
`replacePartition`-failure and crash windows are pinned by
`mtmv_p0/ivm/test_ivm_overwrite_failure_between_the_halves`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
