englefly commented on code in PR #68282:
URL: https://github.com/apache/doris/pull/68282#discussion_r4079935522
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java:
##########
@@ -3904,7 +3904,7 @@ public void truncateTable(String dbName, String
tableName, PartitionNamesInfo pa
oldPartitions = truncateTableInternal(olapTable, newPartitions,
truncateEntireTable, recyclePartitionParamMap, forceDrop,
version, versionTimeMs);
if (truncateEntireTable) {
-
Env.getCurrentEnv().getAnalysisManager().removeTableStats(olapTable.getId());
+
Env.getCurrentEnv().getAnalysisManager().resetTableStats(olapTable);
Review Comment:
Fixed in 19756ddc9de, and the reproduction you describe is now covered by a
deterministic replay-order unit test.
Confirmed the mechanism exactly as you describe it: `resetTableStats()`
created the record when the table had none, so the truncate entry could
re-create a record which a concurrent journaled deletion (`DROP STATS`, the
analyzer's empty table cleanup, the statistics cleaner) had removed. In your
ordering (deletion applied before the truncate reset, but journaled before the
truncate entry) the master ends up without the record while the replay brought
it back, so the record the user dropped reappeared on every other frontend and
after a restart.
Fix: the truncate entry now carries what the truncate did to the record, and
the replay applies exactly that instead of guessing.
* `AnalysisManager.resetTableStats()` reports whether it had to create the
record.
* `InternalCatalog.truncateTable()` writes that into the new
`TruncateTableInfo.tableStatsRecordCreated`.
* `InternalCatalog.replayTruncateTable()` calls the new
`AnalysisManager.replayResetTableStats(table,recordCreated)`, which resets the
record when it exists and creates it only when the truncate created it. A
truncate entry can therefore no longer resurrect a record which another
journaled transition removed, while a table which had no record still gets one
on every frontend, so the rows loaded after the truncation stay accounted for
(an entry written before this change carries no flag and only resets an
existing record).
Deterministic replay-order test, as you asked:
`AnalysisManagerTest.testReplayOfTruncateFollowsTheRecordedStatsTransition`
applies the deletion first and then replays truncate entries for the three
cases (entry which did not create the record -> the record stays absent; entry
which created it -> the record exists with `updatedRows = 0`; entry which did
not create it on a table without a record -> no record).
One residual, stated explicitly rather than left implicit: the transitions
are still not *serialized* with each other. The two `(mutate record, write
journal entry)` pairs of a truncate and of a `DROP STATS` are not protected by
a common lock, so there remains an interleaving where their apply order and
their journal order disagree — it now only requires the record to be absent
when the truncate creates it and the concurrent deletion to be applied after
that creation but journaled before the truncate entry, and in that case the
master ends absent while the replay creates the record. Closing that properly
means coordinating both pairs (writing the deletion entry under the same lock
as the record removal, and the truncate entry under the same lock as the
reset), which touches the deletion paths of `DROP STATS`/analyzer/cleaner and
their journaling; I would rather do that as a separate change than fold it into
this one, and I am happy to send it if you want it here.
--
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]
