yujun777 commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6463338945
### TRUNCATE removes the stats record without journaling the deletion, so
the record a load creates now diverges between the master and its followers
This is the one removal site that is not ordered with a journal entry, and
it is the scenario this PR is about.
**The gap.** A whole table `TRUNCATE` drops the record in memory only —
`InternalCatalog.java:3930` calls
`AnalysisManager.removeTableStats(olapTable.getId())` and never calls
`logDeleteTableStats`, and `InternalCatalog.replayTruncateTable`
(`InternalCatalog.java:3998`) only replaces the partitions and fixes up the
`TabletInvertedIndex`, so a follower replaying `OP_TRUNCATE_TABLE` keeps the
record. The other removals this PR orders are all paired with something the
followers replay: a whole table `DROP STATS` journals `DELETE_TABLE_STATS` (now
via `removeTableStatsAndLog`), and a schema change is paired with the
`FINISHED` entry through `EditLog.java:972`. `TRUNCATE` has no counterpart at
all.
**Why it becomes a defect here.** Before this change a follower could only
ever hold a record that an analyze had produced, so an unjournaled removal
merely left both sides with different *contents* of an old record. Now
`replayUpdateRowsRecord` creates the record of a table that has none, and the
rows it accumulates are journaled: `TransactionState.tableIdToTabletDeltaRows`
is `@SerializedName("deltaRows")` (`TransactionState.java:289`), so
`replayUpsertTransactionState` -> `updateCatalogAfterVisible` recreates the
record on every follower as well. The master's record is therefore no longer
the one the followers have.
For a table that was never analyzed, following the reproduction in the PR
description:
| step | master | follower |
| --- | --- | --- |
| load 50,000,000 rows | record created: `rowCount=0`, `updatedRowsBase=0`,
`updatedRows=5e7` | same (replayed) |
| `TRUNCATE TABLE t` | record removed, **nothing journaled** | record kept,
`updatedRows=5e7` |
| load 3 rows | record recreated: `updatedRows=3` -> scan at 3 | accumulates
-> `updatedRows=50000003` -> scan at 50,000,003 |
Before this PR both frontends agreed on the (wrong) one-row estimate for
such a table; now the master is right and every follower plans a three-row
table as if it still held the rows the truncation removed.
**Impact.** A follower coordinates and plans queries on its own, so in a
multi-FE cluster the same statement gets a different join order and a different
broadcast/shuffle choice depending on which frontend it lands on, for up to
`tablet_stat_update_interval_second` while the new tablets are unreported. The
residue outlives that window: `updatedRows` is only rebased by an analyze, so
the follower keeps reporting the pre-truncate counter as
`Statistics.deltaRowCount`, which `FilterEstimation.java:94` uses as the
population to retry a zero estimate with, until a whole table `ANALYZE`
converges the two sides (`replayUpdateTableStatsStatus` replaces the record).
If the follower is promoted, its divergent record becomes authoritative. Row
visibility and delete bitmaps are unaffected — this is a cardinality-only
defect.
**Suggested fix.** `truncateTable` already holds the table write lock from
before the removal until after `logTruncateTable`, which is exactly the
critical section this PR requires a removal to live in, so it should go through
the new helper:
```java
// InternalCatalog.truncateTable, the truncateEntireTable branch
Env.getCurrentEnv().getAnalysisManager().removeTableStatsAndLog(olapTable);
```
The `writeLock()` inside the helper is reentrant, and a follower replaying
`DELETE_TABLE_STATS` for a record it does not have is a no-op `remove`. The
journal order then becomes `[VISIBLE create] -> [DELETE] -> [VISIBLE create]`,
which matches the order the master applies them in.
For coverage,
`AnalysisManagerTest#testCloudRowUpdateIsSerializedWithTheTableLock` is the
shape to copy — assert the removal and its journal entry appear paired on the
truncate path. `Alter.java:858` looks like another removal of this class (the
original table of an alter that does not swap) and may be worth checking in the
same pass.
--
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]
