englefly commented on PR #68282: URL: https://github.com/apache/doris/pull/68282#issuecomment-5793031747
New commit `11b68bbb685` addresses the two open review threads (the ordering thread `r4078713416` and the MTMV snapshot comment `r4080612135`). No other behaviour changed. **1. Record transition and journal entry are one step** (`r4078713416`) `resetTableStats(table, truncateInfo)` writes `logTruncateTable(...)` inside the same `synchronized (idToTblStats)` block that creates/resets the record and sets `tableStatsRecordCreated`; the new `removeTableStatsAndLog(tableId)` does the removal and `logDeleteTableStats(...)` in the same block, and every removal site outside the replay paths uses it (DROP TABLE, whole-table DROP STATS from the cleaner and the auto collector, ALTER, rename, schema change). Mutation order and journal order are therefore the same order, and the DELETE/TRUNCATE inversion cannot resurrect a dropped record on a follower. Deterministic test: `AnalysisManagerTest#testStatsTransitionAndItsJournalEntryStayOrdered`, plus the existing replay test. **2. Collected row count and baseline are one snapshot** (`r4080612135`) `TableStatsMeta.reset()`/`update()` run under the record monitor and the new `TableStatsMeta.getRowCountWithDeltaRows()` reads the collected count and the delta rows under the same monitor; `StatsCalculator.getOlapTableRowCount()` uses it, so the `MTMVCache.from(..., needLock=false)` path cannot pair the count of one state with the baseline of another one any more. The baseline is also published before the collected counts, for readers which cannot take the monitor. The inflation was reproducible before the change (`100` collected + `150` loaded reported as `250`). Test: `TableStatsMetaTest#testRowCountSnapshotIsCoherent` (real transitions only: truncate, load 150, analyze 100). **Verification on this commit** - `TableStatsMetaTest` 9/9, `AnalysisManagerTest` 18/18 (`mvn test -pl fe-core -am -Dtest='TableStatsMetaTest,AnalysisManagerTest'`, BUILD SUCCESS, checkstyle not skipped). - `./build.sh --fe` SUCCESS; FE + BE restarted from the new build. - End to end: `TRUNCATE TABLE t; INSERT INTO t VALUES (1,1),(2,2),(3,3); explain select * from t;` reports `cardinality=3` (was `1`); steady state and post-ANALYZE stay `5`; a brand new never-analyzed table is unchanged from master (`1`). - Regression suites `test_row_count_after_truncate` and `test_drop_stats_and_truncate`: 1 suite each, 0 failed. **Known gaps, stated explicitly** - `AnalysisManager.updateTableStats` / `updateTableStatsForAlterStats` (analyze completion) still journal the snapshot after the mutation without holding `idToTblStats`; pre-existing, and wrapping it needs the table resolution moved out of the critical section because the truncate path takes the table lock before `idToTblStats`. - A reusable MTMV cache entry which is *built* before a truncation and re-inserted after the invalidation is still possible; it comes from `needLock=false` on-demand planning of a definition whose base tables are not locked, which any concurrent stats change triggers, and locking base tables inside that path would invert the order against the refresh path. - Creating the statistics record at CREATE TABLE time (row count for a never analyzed table) is still not part of this PR; the rationale for leaving it out is in the earlier discussion. -- 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]
