github-actions[bot] commented on code in PR #68282:
URL: https://github.com/apache/doris/pull/68282#discussion_r4063317594
##########
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:
`replayTruncateTable` is now creating/resetting the stats record, but
`Env.replayTruncateTable` still removes it immediately after this method
returns (`Env.java:6770-6772`). On a follower or after checkpoint recovery the
record is therefore absent, so subsequent loads cannot accumulate their deltas
and the reported row count falls back to unknown until another analysis creates
stats. Please remove/condition that outer removal while preserving
compatibility for legacy truncate journals that did not emit the new reset
entry.
##########
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:
Resetting the table-wide counter here does not fence deltas already owned by
a load that started before the truncate. If that load publishes after the
truncate, `DatabaseTransactionMgr.finishCheckPartitionVersion` drops its old
partition commit entries but `updateCatalogAfterVisible` still forwards the
transaction's old tablet-delta map; `AnalysisManager.replayUpdateRowsRecord`
sums it by table ID with no generation check. Those discarded rows are then
reported as new post-truncate rows. Please filter deltas for removed
partitions/tablets (or add a truncate generation fence) before updating this
record.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -130,6 +137,58 @@ public TableStatsMeta(long rowCount, AnalysisInfo
analyzedJob, TableIf table) {
update(analyzedJob, table);
}
+ /**
+ * Create a record for a table which doesn't have one yet, in the state of
an empty table. The rows
+ * loaded into the table are accumulated by {@link
AnalysisManager#replayUpdateRowsRecord}, so a record
+ * has to exist before the first load, otherwise these rows can never be
turned into a row count.
+ */
+ public TableStatsMeta(OlapTable table) {
+ this.ctlId = table.getDatabase().getCatalog().getId();
+ this.ctlName = table.getDatabase().getCatalog().getName();
+ this.dbId = table.getDatabase().getId();
+ this.dbName = table.getDatabase().getFullName();
+ this.tblId = table.getId();
+ this.tblName = table.getName();
+ this.idxId = -1;
+ this.indexesRowCount = buildEmptyIndexRowCount(table);
Review Comment:
This reset is not protected from stale asynchronous producers. An analyze
job started before the truncate can finish afterward and
`AnalysisManager.updateTableStats` will repopulate this same `TableStatsMeta`
with its pre-truncate row/index/column baseline and journal that stale state,
because neither path checks a truncate epoch/table version. The `DropStatsTask`
scheduled after truncate also captures this record and can clear stats set by a
new `ALTER ... SET STATS` if it runs late. Please cancel/fence pre-reset work
and make cleanup/update operations generation-aware.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -105,6 +105,13 @@ public class TableStatsMeta implements Writable,
GsonPostProcessable {
@SerializedName("irc")
private ConcurrentMap<Long, Long> indexesRowCount = new
ConcurrentHashMap<>();
+ // The value of updatedRows when indexesRowCount was collected, i.e. the
number of rows the collected
+ // row count already includes. The rows loaded after that point are the
delta row count of the table.
+ // It is kept here, and not derived from colToColStatsMeta, so that
dropping the column statistics of
+ // the table doesn't lose it. -1 means no row count has ever been
collected from the table.
+ @SerializedName("updatedRowsBase")
Review Comment:
`updatedRowsBase` is table-wide, but this assignment runs even when the job
did not collect the base-index row count. For example, an MV-only analysis
updates only the MV entry, and a named-partition analysis can finish without
populating `indexesRowCount`; after 50 rows load, the stored base index remains
100 while `updatedRowsBase` becomes 150, so the unknown-BE fallback returns
`100 + (150-150) = 100` instead of 150. Track the baseline per index or advance
the base-index baseline only when it was actually refreshed.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1487,6 +1487,38 @@ public void removeTableStats(long tableId) {
}
}
+ /**
+ * TRUNCATE TABLE removes all the data of the table, but the table itself
stays and can be loaded
+ * immediately. The stats record must be kept, otherwise the row count of
the newly loaded data can
+ * never be reported: the backends report the row count of the new empty
tablets with a delay of up to
+ * {@code tablet_stat_update_interval_second}, and without a record there
is nothing to accumulate the
+ * loaded rows into. So reset the record to the state of an empty table
instead of removing it.
+ */
+ public void resetTableStats(OlapTable table) {
+ // Keep the followers and the checkpoint image consistent with the
master.
+ logCreateTableStats(resetTableStatsInternal(table));
Review Comment:
This call writes a separate `OP_UPDATE_TABLE_STATS` entry before
`InternalCatalog` writes the `OP_TRUNCATE_TABLE` entry. `EditLog.logEdit`
explicitly has no cross-entry atomicity, so a FE crash after this write but
before the truncate journal is durable replays a zeroed stats record onto the
old partitions/data. The next planner fallback can therefore undercount
existing rows until reanalysis. Persist the reset as part of the truncate
operation or make recovery ignore/roll back an unpaired reset.
--
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]
