You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "englefly (via GitHub)" <gi...@apache.org> on 2026/10/13 12:34:05 UTC
[PR] [fix](fe) Create the table stats record when a load finishes [doris]
englefly opened a new pull request, #69028:
URL: https://github.com/apache/doris/pull/69028
An alternative implementation of the fix discussed in #68282, with a much smaller change: instead of keeping
and resetting the statistics record across `TRUNCATE TABLE` (which also had to order every record transition
with its journal entry), the record is recreated by the load itself, so a truncation keeps removing it exactly
as it does today.
### What problem does this PR solve?
Issue Number: None (no upstream issue filed; the case is described below)
Problem Summary:
Reproduction on a single bucket duplicate key table:
CREATE TABLE t (k INT, v INT) DUPLICATE KEY(k) DISTRIBUTED BY HASH(k) BUCKETS 1;
INSERT INTO t VALUES (1,1),(2,2),(3,3),(4,4),(5,5);
TRUNCATE TABLE t;
INSERT INTO t VALUES (1,1),(2,2),(3,3);
EXPLAIN SELECT * FROM t; -- reports cardinality=1, although the table holds 3 rows
TRUNCATE TABLE replaces the partitions and tablets of the table with new ones and removes its statistics
record. The backends report the row count of the new tablets only after the delay of
`tablet_stat_update_interval_second` (60s by default), and a partition reports no row count at all until its
first load becomes visible (`DatabaseTransactionMgr` sets `rowCountReported` to false for the first
transaction of a partition), so during that interval `OlapTable.getRowCountForIndex(..., strict = true)`
returns -1 for the whole table. The only other source of a row count is the statistics record of the table,
and the truncation had just removed it, so the planner clamps the table to one row: joins are ordered and
distributed for a table that looks empty instead of for the rows that were just loaded.
The load now recreates the record itself. While the loading transaction becomes visible,
`AnalysisManager.replayUpdateRowsRecord()` creates the record of a table which has none in the state of a
table whose row count has never been collected, and accumulates the rows it reports into it, so the planner
reads them instead of -1. The record is created with a baseline of 0 (`updatedRowsBase`) and a collected row
count of 0 for the base index, so every row loaded afterwards is a delta row and the reported row count is
the number of rows the table holds.
The creation happens in `updateUpdatedRows()`, which is the path the master walks when the loading
transaction becomes visible (`DatabaseTransactionMgr.updateCatalogAfterVisible`) and the path a follower
walks while it replays the transaction state (`replayUpsertTransactionState`), so every frontend ends up with
the record. It needs no edit log entry of its own and does not touch the transaction state machine: the
record is a function of the transaction state the journal already carries. The whole table `DROP STATS`, the
schema changes and the truncation keep removing the record as before, and the next load recreates it. A
partition truncation is deliberately left out: the rows it reports are the rows it removed, so it does not
create a record.
Two related fixes come with it, because the record is now the row count source in more cases:
- the delta is only added to an index which keeps one row per base row
(`TableStatsMeta.keepsOneRowPerBaseRow`): an aggregating rollup, an index which merges the rows of a unique
key table and a filtered materialized index have their own row count and must not be reported with the
physical rows of the base index, and a row count which was never collected is not combined with a delta;
- `StatisticsMetricCollector` decided "not analyzed" by the absence of a record. A record now exists for a
loaded table which was never analyzed, so it decides by the analyze timestamp instead.
### Known limitation
A record which is created for a table that already holds rows, for instance right after a whole table
`DROP STATS` or after a schema change (the truncation case starts from an empty table), only knows the rows
loaded after that point, so the table is estimated with them until the backends report the tablets or an
analysis collects the row count. It is still closer than the unknown row count it replaces: a merge key
table reports the physical row count in that window, which is an upper bound of its logical row count, as it
does in the steady state today.
### Release note
None
### Check List (For Author)
- Test: new unit tests in `AnalysisManagerTest` (a load creates the record and accumulates its rows, a load
which reports the rows by table id creates it as well, a replayed load creates the same record, an existing
analyzed record is only accumulated, and an unresolvable table creates none) and in `TableStatsMetaTest`
(the record of a table whose row count was never collected reports the loaded rows; the baseline survives
dropping the column statistics; a legacy record derives it from the column statistics; a stale analyze
snapshot and its baseline cancel out; a user injected row count is not counted twice; the reader of the
snapshot never pairs two states; a row preserving rollup reports the loaded rows while an aggregating one
and a filtered one do not). `AnalysisManagerTest` 21/21, `TableStatsMetaTest` 12/12 (checkstyle not
skipped). Regression: the new suite `test_row_count_after_truncate` (the truncate case, an aggregate rollup
and a projection rollup) and the existing `test_drop_stats_and_truncate` both pass. Manual test on a local
FE and BE cluster with the reproduction above: the scan reports cardinality=3 right after the load instead
of 1, a never analyzed table which receives 4 rows reports 4 instead of 1, and `SHOW TABLE STATS` shows the
accumulated rows with an empty analyze time.
- Behavior changed: Yes. A table whose statistics record was removed reports the rows its loads accumulate
instead of an unknown row count (planned as one row) while the backends have not reported the new tablets
yet; the "not analyzed" metric counts a table by its analyze timestamp.
- Does this need documentation: No
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6163362959
# FE Regression Coverage Report
Increment line coverage ` 36.93% (89/241)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_e2ce99b43fe1332fbad22f00c5019273a29bf94c_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_e2ce99b43fe1332fbad22f00c5019273a29bf94c_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4355140831
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/repository/StatisticsCleaner.java:
##########
@@ -140,6 +140,8 @@ private void clearTableStats() {
}
LOG.info("Table {}.{}.{} with id {} not exist, remove its table stats record.",
stats.ctlName, stats.dbName, stats.tblName, id);
+ // No table lock: the table is gone, so no load can recreate the record of this id while the
Review Comment:
[P2] Order cleaner deletion with recovery of the same table ID
`RECOVER TABLE` re-registers the recycled `Table` with its original ID, so the absence observed here does not prevent load-driven recreation. The cleaner can capture `stats` before DROP, observe the dropped table, remove its record, and pause before journaling DELETE. Run `RECOVER TABLE t; TRUNCATE TABLE t; INSERT ...` with N rows, then resume the cleaner: the master retains the load-created record, while replay applies RECOVER -> TRUNCATE -> VISIBLE (create N) -> DELETE (remove). With the replacement tablets still unreported, `Project(*) -> OlapScan(t)` consequently estimates N on the master but 1 after replay/failover. The old get-only load path left both records absent. This is a separate remover from the existing DROP STATS/schema threads. Serialize the cleaner's absence check and removal/journal with recovery/load publication, and cover this interleaving with barriers.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4312382722
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -194,6 +273,21 @@ public void update(AnalysisInfo analyzedJob, TableIf tableIf) {
if (tableIf != null) {
if (tableIf instanceof OlapTable) {
OlapTable olapTable = (OlapTable) tableIf;
+ // The collected row counts published below already include the rows which had been loaded when
+ // the job was built, remember how many they were, they are not delta rows. The baseline may
+ // only advance together with the collected base index row count, an analysis of another index
+ // (a materialized view) doesn't touch it.
+ // NOTICE: the baseline is published before the collected counts. They are both read without
+ // the table lock by the planner, and a reader which pairs the collected count of this analysis
+ // with the baseline of the previous one counts the rows loaded since that previous analysis
+ // twice.
+ if (analyzedJob.indexesRowCount.containsKey(olapTable.getBaseIndexId())) {
+ // A row count supplied by the user replaces the collected one and describes the table as
+ // of now, so the rows loaded so far are part of it and the baseline has to move to the
+ // current value. Keeping the older baseline would count those rows again once DROP STATS
+ // clears userInjected.
+ updatedRowsBase.set(analyzedJob.userInject ? updatedRows.get() : analyzedJob.updateRows);
Review Comment:
Agreed that the two values describe different snapshots, but this one is pre-existing rather than a regression from this change: before the record-creation work the baseline was derived from `ColStatsMeta.updatedRows`, which is also `analyzedJob.updateRows`, captured in `AnalysisManager#buildAnalysisInfoForTbl` (line 428) when the job is built, while the count comes from the task's `COUNT(1)` executed later. `updatedRowsBase` keeps exactly the same capture point, so master computes the same wrong estimate (`150 + (150 - 100) = 200`).
Tracked separately in #69281. The fix is to capture the load counter when the analyze query collects the row count (after the query, so the residual race biases to an under-count instead of a double count) and use it as the baseline, plus a deterministic "build job -> load -> count" test. The test added here only covers a stale snapshot delivered after the record was recreated, where the baseline and the collected count cancel out.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4309991283
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1366,52 +1405,52 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
// Record : TableId -> (TabletId -> update rows)
if (event.getTabletRecords() != null) {
for (Entry<Long, Map<Long, Long>> record : event.getTabletRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
- if (statsStatus != null) {
- Optional<Database> dbOption = catalog.getDb(event.getDbId());
- if (!dbOption.isPresent()) {
- LOG.warn("Database {} does not exist.", event.getDbId());
- continue;
- }
- Database database = dbOption.get();
- Optional<Table> tableOption = database.getTable(record.getKey());
- if (!tableOption.isPresent()) {
- LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
- continue;
- }
- Table table = tableOption.get();
- if (!(table instanceof OlapTable)) {
- continue;
- }
- OlapTable olapTable = (OlapTable) table;
- short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
- Map<Long, Long> tabletRows = record.getValue();
- if (tabletRows == null || tabletRows.isEmpty()) {
- LOG.info("Tablet row count map is empty");
- continue;
- }
- long rowsForAllReplica = 0;
- for (Entry<Long, Long> entry : tabletRows.entrySet()) {
- rowsForAllReplica += entry.getValue();
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table id {}, tablet id {}, row count {}",
- record.getKey(), entry.getKey(), entry.getValue());
- }
- }
- long tableUpdateRows = rowsForAllReplica / replicaNum;
- LOG.info("Update rows for table {} is {}, replicaNum is {}, "
- + "rows for all replica {}, tablets count {}",
- olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
- statsStatus.updatedRows.addAndGet(tableUpdateRows);
- if (StatisticsUtil.enablePartitionAnalyze()) {
- updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ Optional<Database> dbOption = catalog.getDb(event.getDbId());
+ if (!dbOption.isPresent()) {
+ LOG.warn("Database {} does not exist.", event.getDbId());
+ continue;
+ }
+ Database database = dbOption.get();
+ Optional<Table> tableOption = database.getTable(record.getKey());
+ if (!tableOption.isPresent()) {
+ LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
+ continue;
+ }
+ Table table = tableOption.get();
+ if (!(table instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ TableStatsMeta statsStatus = findOrCreateTableStats(olapTable);
+ short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
+ Map<Long, Long> tabletRows = record.getValue();
+ if (tabletRows == null || tabletRows.isEmpty()) {
+ LOG.info("Tablet row count map is empty");
+ continue;
+ }
+ long rowsForAllReplica = 0;
+ for (Entry<Long, Long> entry : tabletRows.entrySet()) {
+ rowsForAllReplica += entry.getValue();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Table id {}, tablet id {}, row count {}",
+ record.getKey(), entry.getKey(), entry.getValue());
}
}
+ long tableUpdateRows = rowsForAllReplica / replicaNum;
+ LOG.info("Update rows for table {} is {}, replicaNum is {}, "
+ + "rows for all replica {}, tablets count {}",
+ olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
+ statsStatus.updatedRows.addAndGet(tableUpdateRows);
Review Comment:
这是另一个bug, 和本pr 无关. 临时分区的行数不应该记录在updatedRows里.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4355526082
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/repository/StatisticsCleaner.java:
##########
@@ -140,6 +140,8 @@ private void clearTableStats() {
}
LOG.info("Table {}.{}.{} with id {} not exist, remove its table stats record.",
stats.ctlName, stats.dbName, stats.tblName, id);
+ // No table lock: the table is gone, so no load can recreate the record of this id while the
Review Comment:
Fixed in 4c792cff11f.
The cleaner now takes the **write lock of the recycled table** — the very instance `RECOVER TABLE`
registers again, and the lock a load holds in `finishTransaction` until it recreated the record — around
the removal **and** its journal entry, and it checks the catalog again under that lock:
- if the deletion is applied before the recovery/load, the load journals its creation after this
`DELETE_TABLE_STATS`, so the replay order matches the master;
- if the load already recreated the record, that record is the one which is removed, and the replay
removes it as well;
- a table which was recovered between the check and the lock keeps its statistics.
A record whose table is gone for good (not in the recycle bin, hence not recoverable) keeps the old
unlocked removal. `CatalogRecycleBin#getRecycledTable` exposes the recycled instance; the previous
"no lock needed" comment was wrong because `RECOVER TABLE` re-registers the table with its original id.
Tests: `StatisticsCleanerTest#testRecycledTableStatsAreRemovedUnderTheTableLock` (the removal and the
journal entry happen between `writeLock` and `writeUnlock`) and
`StatisticsCleanerTest#testTableRecoveredBeforeTheRemovalKeepsItsStats` (a table which is back in the
catalog under the lock is left alone). `StatisticsCleanerTest` 3/3, checkstyle passes.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6355949419
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6160255591
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4309726925
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -268,6 +382,29 @@ public long getBaseIndexDeltaRowCount(OlapTable table) {
return updatedRows.get() - maxUpdateRows;
}
+ /**
+ * The row count of the index together with the rows loaded since it was collected, i.e. the row count of
+ * the table. Both are read by the planner without the table lock, for instance while it plans a direct
+ * scan of a materialized view, so they have to come from the same state of this record: a collected row
+ * count paired with the baseline of another analysis would count rows twice or miss them. The rows loaded
+ * while this call runs are not part of the snapshot, whichever state reads them accumulates them in
+ * {@link #updatedRows} and reports them as delta rows.
+ */
+ public synchronized long getRowCountWithDeltaRows(OlapTable table, long indexId) {
+ long rowCount = getRowCount(indexId);
+ if (!keepsOneRowPerBaseRow(table, indexId)) {
+ // The index aggregates, or it merges the rows of a unique key table: it has its own, smaller row
+ // count, and the rows loaded into the base index would overstate it.
+ return rowCount;
+ }
+ if (rowCount == TableIf.UNKNOWN_ROW_COUNT) {
+ // The index has no collected row count yet, neither the row count of the table nor a delta row
+ // count can be derived from it. The planner treats the unknown row count as no information.
+ return rowCount;
+ }
+ return rowCount + getBaseIndexDeltaRowCount(table);
Review Comment:
只有在使用set stats 这条测试命令时才会出现多统计行数的情况, 不是用户场景,不用处理
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4355069153
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1298,12 +1344,12 @@ public void updateUpdatedRows(Map<Long, Long> partitionToUpdateRows, long dbId,
}
// Invoke this for cloud version load.
- public void updateUpdatedRows(Map<Long, Long> updatedRows) {
+ public void updateUpdatedRows(Map<Long, Long> updatedRows, long dbId) {
try {
if (!Env.getCurrentEnv().isMaster() || Env.isCheckpointThread()) {
return;
}
- UpdateRowsEvent updateRowsEvent = new UpdateRowsEvent(updatedRows);
+ UpdateRowsEvent updateRowsEvent = new UpdateRowsEvent(dbId, updatedRows);
Review Comment:
Fixed in 50074783a42.
`AnalysisManager.updateUpdatedRows(Map, dbId)` — the cloud report path — now holds the table write
lock of the reported tables while it applies **and** journals the rows, i.e. the same lock which a
whole table `DROP STATS` takes to remove the record and journal the deletion. The journal order is
then always the order in which the mutations are applied, so both interleavings converge:
- report first: journal `[UPDATE_ROWS][DELETE]` -> master create(N) then remove, replay create(N) then remove
- DROP first: journal `[DELETE][UPDATE_ROWS]` -> master remove then create(N), replay remove then create(N)
The tables are locked in id order with the 10 second timeout `finishTransaction` uses; when the lock
cannot be acquired the report is dropped with a warning and the row count stays unknown until the
backends report it.
Test: `AnalysisManagerTest#testCloudRowUpdateIsSerializedWithTheTableLock` holds the table write lock,
reports rows from another thread and asserts that neither the record nor the journal entry appears
while the lock is held (and that both appear after the unlock). It fails without the change with
"the report was applied while the table was locked".
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1355,7 +1401,7 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
InternalCatalog catalog = Env.getCurrentInternalCatalog();
if (event.getRecords() != null) {
for (Entry<Long, Long> record : event.getRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
+ TableStatsMeta statsStatus = findOrCreateTableStats(catalog, event.getDbId(), record.getKey());
Review Comment:
Fixed in d1aca0b7213.
The partition generation of the report is now validated: `CloudGlobalTransactionMgr.afterCommitTxnResp`
hands the `<table_id, partition_id>` pairs which `CommitTxnResponse` already carries to
`AnalysisManager.updateUpdatedRows`, which drops a table's rows when any of the partitions the report
counted no longer belongs to the table (a whole table `TRUNCATE` replaces the partitions and keeps the
table id). The check runs inside the table write lock the report takes, so the generation cannot change
between the check and the application, and a dropped report is neither applied nor journaled — so the
record the next load recreates only counts the rows of the current partitions.
Tests: `AnalysisManagerTest#testReportOfAPartitionRemovedByTruncateIsNotApplied` (a delayed report of
100 rows for the replaced partition is ignored; the reload's 3 rows recreate the record with 3 rows,
not 103) and `AnalysisManagerTest#testReportOfRemovedPartitionsIsRecognized`.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1366,52 +1412,52 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
// Record : TableId -> (TabletId -> update rows)
if (event.getTabletRecords() != null) {
for (Entry<Long, Map<Long, Long>> record : event.getTabletRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
- if (statsStatus != null) {
- Optional<Database> dbOption = catalog.getDb(event.getDbId());
- if (!dbOption.isPresent()) {
- LOG.warn("Database {} does not exist.", event.getDbId());
- continue;
- }
- Database database = dbOption.get();
- Optional<Table> tableOption = database.getTable(record.getKey());
- if (!tableOption.isPresent()) {
- LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
- continue;
- }
- Table table = tableOption.get();
- if (!(table instanceof OlapTable)) {
- continue;
- }
- OlapTable olapTable = (OlapTable) table;
- short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
- Map<Long, Long> tabletRows = record.getValue();
- if (tabletRows == null || tabletRows.isEmpty()) {
- LOG.info("Tablet row count map is empty");
- continue;
- }
- long rowsForAllReplica = 0;
- for (Entry<Long, Long> entry : tabletRows.entrySet()) {
- rowsForAllReplica += entry.getValue();
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table id {}, tablet id {}, row count {}",
- record.getKey(), entry.getKey(), entry.getValue());
- }
- }
- long tableUpdateRows = rowsForAllReplica / replicaNum;
- LOG.info("Update rows for table {} is {}, replicaNum is {}, "
- + "rows for all replica {}, tablets count {}",
- olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
- statsStatus.updatedRows.addAndGet(tableUpdateRows);
- if (StatisticsUtil.enablePartitionAnalyze()) {
- updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ Optional<Database> dbOption = catalog.getDb(event.getDbId());
+ if (!dbOption.isPresent()) {
+ LOG.warn("Database {} does not exist.", event.getDbId());
+ continue;
+ }
+ Database database = dbOption.get();
+ Optional<Table> tableOption = database.getTable(record.getKey());
+ if (!tableOption.isPresent()) {
+ LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
+ continue;
+ }
+ Table table = tableOption.get();
+ if (!(table instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ TableStatsMeta statsStatus = findOrCreateTableStats(olapTable);
Review Comment:
Fixed in b01c9770473.
The removal moved into the same critical section which journals `FINISHED`, i.e. before
`tbl.writeUnlock()`, so a load can no longer become visible in between: either the load ran before the
schema change finished (the record it updated is removed by the job on both the master and the replay)
or after (the record it recreates exists on both). `removeTableStatsOnFinished` asserts that the table
write lock is held, so the call cannot be moved out of the critical section again; dropping the
persisted statistics and invalidating the caches (`dropStats`) stays outside the lock because it does
not touch the record.
Test: `SchemaChangeJobV2StatsTest#testRemoveTableStatsOnFinishedRequiresTheTableWriteLock` (the removal
is rejected without the table write lock and removes the record with it).
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6363021585
<details>
<summary>ClickBench: <b>Total hot run time: 24.38 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 4c792cff11ff0151e7af0174bf908dcb0d8a056c, data reload: false
query1 0.01 0.01 0.00
query2 0.10 0.05 0.05
query3 0.27 0.09 0.09
query4 1.61 0.10 0.11
query5 0.23 0.23 0.25
query6 1.17 0.68 0.68
query7 0.04 0.01 0.00
query8 0.06 0.04 0.04
query9 0.41 0.35 0.37
query10 0.57 0.56 0.59
query11 0.21 0.15 0.15
query12 0.18 0.15 0.15
query13 0.50 0.49 0.50
query14 1.03 1.01 1.01
query15 0.63 0.60 0.60
query16 0.34 0.34 0.36
query17 1.12 1.15 1.12
query18 0.21 0.19 0.19
query19 2.07 2.06 1.99
query20 0.02 0.01 0.01
query21 15.49 0.19 0.14
query22 4.89 0.05 0.05
query23 16.25 0.32 0.12
query24 3.05 0.52 0.43
query25 0.12 0.05 0.06
query26 1.39 0.23 0.16
query27 0.07 0.06 0.06
query28 3.25 0.77 0.36
query29 12.49 4.13 3.23
query30 0.28 0.13 0.11
query31 2.78 0.55 0.33
query32 3.23 0.58 0.50
query33 3.27 3.29 3.21
query34 15.30 3.99 3.39
query35 3.35 3.32 3.32
query36 0.55 0.42 0.41
query37 0.09 0.06 0.06
query38 0.05 0.04 0.04
query39 0.03 0.03 0.03
query40 0.18 0.14 0.14
query41 0.08 0.03 0.02
query42 0.04 0.03 0.03
query43 0.04 0.04 0.04
Total cold run time: 97.05 s
Total hot run time: 24.38 s
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6342295103
<details>
<summary>ClickBench: <b>Total hot run time: 24.4 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 70184f06f222076c5ba0b094186ccf27d09fc006, data reload: false
query1 0.00 0.00 0.01
query2 0.10 0.06 0.05
query3 0.27 0.08 0.08
query4 1.61 0.11 0.10
query5 0.23 0.24 0.26
query6 1.17 0.67 0.67
query7 0.04 0.01 0.01
query8 0.05 0.04 0.04
query9 0.40 0.31 0.35
query10 0.57 0.57 0.58
query11 0.21 0.15 0.14
query12 0.19 0.15 0.14
query13 0.52 0.51 0.50
query14 1.03 1.01 1.02
query15 0.64 0.61 0.62
query16 0.35 0.35 0.34
query17 1.16 1.16 1.14
query18 0.19 0.18 0.19
query19 2.07 2.02 2.02
query20 0.02 0.02 0.01
query21 15.52 0.19 0.14
query22 4.81 0.05 0.05
query23 16.32 0.32 0.13
query24 2.88 0.51 0.43
query25 0.12 0.06 0.06
query26 0.88 0.22 0.15
query27 0.05 0.05 0.04
query28 2.91 0.82 0.39
query29 12.49 4.24 3.22
query30 0.28 0.15 0.10
query31 2.77 0.55 0.34
query32 3.23 0.60 0.52
query33 3.28 3.18 3.15
query34 15.31 4.00 3.41
query35 3.33 3.31 3.35
query36 0.56 0.45 0.44
query37 0.09 0.06 0.06
query38 0.05 0.04 0.04
query39 0.04 0.03 0.03
query40 0.17 0.15 0.15
query41 0.08 0.02 0.02
query42 0.04 0.03 0.03
query43 0.05 0.04 0.03
Total cold run time: 96.08 s
Total hot run time: 24.4 s
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6362780244
<details>
<summary>TPC-H: <b>Total hot run time: 28451 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 4c792cff11ff0151e7af0174bf908dcb0d8a056c, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17616 5176 5056 5056
q2 2154 326 292 292
q3 10162 1400 822 822
q4 4682 464 342 342
q5 7448 809 542 542
q6 181 183 151 151
q7 757 799 597 597
q8 9256 1314 1094 1094
q9 5472 4164 4175 4164
q10 6829 1292 1026 1026
q11 435 241 225 225
q12 644 394 275 275
q13 18145 3606 3004 3004
q14 277 262 251 251
q15 q16 730 718 689 689
q17 1194 745 578 578
q18 6962 6185 6126 6126
q19 1129 946 604 604
q20 389 338 229 229
q21 5447 2128 2247 2128
q22 341 291 256 256
Total cold run time: 100250 ms
Total hot run time: 28451 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 5580 5600 5590 5590
q2 697 580 535 535
q3 5231 5387 4936 4936
q4 1992 2174 1541 1541
q5 5300 5177 5715 5177
q6 266 200 151 151
q7 2325 2142 1871 1871
q8 3129 2932 2671 2671
q9 7989 8106 8077 8077
q10 3702 3667 3196 3196
q11 645 426 396 396
q12 672 689 510 510
q13 3536 3742 3230 3230
q14 318 305 272 272
q15 q16 687 712 652 652
q17 8258 7440 7261 7261
q18 12958 12205 12891 12205
q19 889 773 779 773
q20 2134 2125 1909 1909
q21 5600 4578 4711 4578
q22 518 451 409 409
Total cold run time: 72426 ms
Total hot run time: 65940 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6260135030
# FE Regression Coverage Report
Increment line coverage ` 3.96% (89/2246)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_e2ce99b43fe1332fbad22f00c5019273a29bf94c_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_e2ce99b43fe1332fbad22f00c5019273a29bf94c_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6260659798
# FE Regression Coverage Report
Increment line coverage ` 3.96% (89/2246)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_e2ce99b43fe1332fbad22f00c5019273a29bf94c_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_e2ce99b43fe1332fbad22f00c5019273a29bf94c_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4311337857
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java:
##########
@@ -384,11 +384,19 @@ public Statistics visitLogicalFilter(LogicalFilter<? extends Plan> filter, Void
* returns the sum of deltaRowCount for all selected partitions or for the table.
*/
private long computeDeltaRowCount(OlapScan olapScan) {
+ OlapTable olapTable = olapScan.getTable();
+ // The delta rows are the rows loaded into the base index which the collected base index row count
+ // doesn't include yet. They are not the rows of an index which aggregates them: such an index has its
+ // own (usually much smaller) row count, so the delta only applies to a scan of an index which keeps
+ // one row per base row.
+ if (!TableStatsMeta.keepsOneRowPerBaseRow(olapTable, olapScan.getSelectedIndexId())) {
+ return 0;
Review Comment:
Good catch, fixed in dc2d63c6987.
`computeDeltaRowCount()` no longer checks `keepsOneRowPerBaseRow`. The value it returns is not the row count of the scanned index: it only feeds `Statistics.deltaRowCount`, which `FilterEstimation.estimate()` uses as the population to retry a zero estimate with, and stale column statistics are not index specific. The check stays in `TableStatsMeta#getRowCountWithDeltaRows()`, the only place where the delta is added to the collected row count of an index, and the javadoc of `computeDeltaRowCount()` now spells that split out so the check does not get re-added by accident.
For the analyze/load/out-of-range-filter case, `StatsCalculatorTest#testDeltaRowCountIsReportedForNonRowPreservingIndex` asserts that a scan of a non-row-preserving index (an AGG rollup) over a record created by a load still reports the delta rows, i.e. the population the retry needs; the recovery itself is already covered by the existing `FilterEstimationTest#testDeltaRow`. Both compile and the new test passes locally.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4312193993
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java:
##########
@@ -384,11 +384,19 @@ public Statistics visitLogicalFilter(LogicalFilter<? extends Plan> filter, Void
* returns the sum of deltaRowCount for all selected partitions or for the table.
*/
private long computeDeltaRowCount(OlapScan olapScan) {
+ OlapTable olapTable = olapScan.getTable();
+ // The delta rows are the rows loaded into the base index which the collected base index row count
+ // doesn't include yet. They are not the rows of an index which aggregates them: such an index has its
+ // own (usually much smaller) row count, so the delta only applies to a scan of an index which keeps
+ // one row per base row.
+ if (!TableStatsMeta.keepsOneRowPerBaseRow(olapTable, olapScan.getSelectedIndexId())) {
+ return 0;
Review Comment:
done
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4355718537
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/transaction/CloudGlobalTransactionMgr.java:
##########
@@ -545,7 +554,7 @@ public void afterCommitTxnResp(CommitTxnResponse commitTxnResponse, List<TabletC
}
}
Env env = Env.getCurrentEnv();
- env.getAnalysisManager().updateUpdatedRows(updatedRows);
+ env.getAnalysisManager().updateUpdatedRows(updatedRows, dbId, reportedPartitions);
Review Comment:
[P2] Supply row counters for the other cloud load producers
On a cloud DUP table, `TRUNCATE TABLE t; BEGIN; INSERT INTO t SELECT number FROM numbers("number"="3"); COMMIT;` still leaves `Project(*) -> OlapScan(t)` estimated at 1 before tablet statistics arrive: `commitTransactionWithSubTxns()` puts base-tablet IDs only in `SubTxnInfo`, but meta-service `commit_txn_with_sub_txn()` passes the empty top-level `request->base_tablet_ids()` to `calc_table_stats()`. Its response therefore has no `updated_row_count`, so the new call here receives an empty `updatedRows` map and creates no record. Non-MoW/non-TSO BE-direct stream loads with `enable_stream_load_commit_txn_on_be=true` have the same gap because `CloudMetaMgr::commit_txn()` supplies no base-tablet IDs either. These paths retain the original symptom despite the new cloud recreation path. Carry the required tablet IDs/counters through both producers and add coverage using their actual commit responses.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6161097857
<details>
<summary>TPC-H: <b>Total hot run time: 29273 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit e2ce99b43fe1332fbad22f00c5019273a29bf94c, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17630 5619 5561 5561
q2 2136 317 271 271
q3 10194 1431 851 851
q4 4682 482 364 364
q5 7442 790 547 547
q6 191 186 157 157
q7 772 798 594 594
q8 9239 1355 1081 1081
q9 5698 4475 4482 4475
q10 6813 1303 1038 1038
q11 456 257 240 240
q12 632 404 286 286
q13 18078 3072 2387 2387
q14 278 274 248 248
q15 q16 726 733 678 678
q17 1190 741 564 564
q18 7197 6230 6212 6212
q19 1103 966 619 619
q20 402 343 234 234
q21 5632 2564 2662 2564
q22 399 364 302 302
Total cold run time: 100890 ms
Total hot run time: 29273 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 6665 6640 6851 6640
q2 720 632 579 579
q3 5524 5762 5221 5221
q4 2055 2162 1600 1600
q5 5700 5590 5519 5519
q6 317 208 150 150
q7 2326 2028 1764 1764
q8 3147 2787 2741 2741
q9 8319 8157 8130 8130
q10 3768 3590 3274 3274
q11 592 399 386 386
q12 673 702 516 516
q13 2760 3067 2403 2403
q14 286 295 269 269
q15 q16 671 673 593 593
q17 7863 6990 6960 6960
q18 13193 12256 13051 12256
q19 922 804 800 800
q20 2183 2146 1940 1940
q21 5848 4730 4804 4730
q22 520 451 394 394
Total cold run time: 74052 ms
Total hot run time: 66865 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4309991283
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1366,52 +1405,52 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
// Record : TableId -> (TabletId -> update rows)
if (event.getTabletRecords() != null) {
for (Entry<Long, Map<Long, Long>> record : event.getTabletRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
- if (statsStatus != null) {
- Optional<Database> dbOption = catalog.getDb(event.getDbId());
- if (!dbOption.isPresent()) {
- LOG.warn("Database {} does not exist.", event.getDbId());
- continue;
- }
- Database database = dbOption.get();
- Optional<Table> tableOption = database.getTable(record.getKey());
- if (!tableOption.isPresent()) {
- LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
- continue;
- }
- Table table = tableOption.get();
- if (!(table instanceof OlapTable)) {
- continue;
- }
- OlapTable olapTable = (OlapTable) table;
- short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
- Map<Long, Long> tabletRows = record.getValue();
- if (tabletRows == null || tabletRows.isEmpty()) {
- LOG.info("Tablet row count map is empty");
- continue;
- }
- long rowsForAllReplica = 0;
- for (Entry<Long, Long> entry : tabletRows.entrySet()) {
- rowsForAllReplica += entry.getValue();
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table id {}, tablet id {}, row count {}",
- record.getKey(), entry.getKey(), entry.getValue());
- }
- }
- long tableUpdateRows = rowsForAllReplica / replicaNum;
- LOG.info("Update rows for table {} is {}, replicaNum is {}, "
- + "rows for all replica {}, tablets count {}",
- olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
- statsStatus.updatedRows.addAndGet(tableUpdateRows);
- if (StatisticsUtil.enablePartitionAnalyze()) {
- updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ Optional<Database> dbOption = catalog.getDb(event.getDbId());
+ if (!dbOption.isPresent()) {
+ LOG.warn("Database {} does not exist.", event.getDbId());
+ continue;
+ }
+ Database database = dbOption.get();
+ Optional<Table> tableOption = database.getTable(record.getKey());
+ if (!tableOption.isPresent()) {
+ LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
+ continue;
+ }
+ Table table = tableOption.get();
+ if (!(table instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ TableStatsMeta statsStatus = findOrCreateTableStats(olapTable);
+ short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
+ Map<Long, Long> tabletRows = record.getValue();
+ if (tabletRows == null || tabletRows.isEmpty()) {
+ LOG.info("Tablet row count map is empty");
+ continue;
+ }
+ long rowsForAllReplica = 0;
+ for (Entry<Long, Long> entry : tabletRows.entrySet()) {
+ rowsForAllReplica += entry.getValue();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Table id {}, tablet id {}, row count {}",
+ record.getKey(), entry.getKey(), entry.getValue());
}
}
+ long tableUpdateRows = rowsForAllReplica / replicaNum;
+ LOG.info("Update rows for table {} is {}, replicaNum is {}, "
+ + "rows for all replica {}, tablets count {}",
+ olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
+ statsStatus.updatedRows.addAndGet(tableUpdateRows);
Review Comment:
这是另一个bug, 和本pr 无关. 临时分区的行数不应该记录在updatedRows里.
单独提了一个issue https://github.com/apache/doris/issues/69262
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4345396050
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -268,6 +382,29 @@ public long getBaseIndexDeltaRowCount(OlapTable table) {
return updatedRows.get() - maxUpdateRows;
}
+ /**
+ * The row count of the index together with the rows loaded since it was collected, i.e. the row count of
+ * the table. Both are read by the planner without the table lock, for instance while it plans a direct
+ * scan of a materialized view, so they have to come from the same state of this record: a collected row
+ * count paired with the baseline of another analysis would count rows twice or miss them. The rows loaded
+ * while this call runs are not part of the snapshot, whichever state reads them accumulates them in
+ * {@link #updatedRows} and reports them as delta rows.
+ */
+ public synchronized long getRowCountWithDeltaRows(OlapTable table, long indexId) {
+ long rowCount = getRowCount(indexId);
+ if (!keepsOneRowPerBaseRow(table, indexId)) {
+ // The index aggregates, or it merges the rows of a unique key table: it has its own, smaller row
+ // count, and the rows loaded into the base index would overstate it.
+ return rowCount;
+ }
+ if (rowCount == TableIf.UNKNOWN_ROW_COUNT) {
Review Comment:
如果一个rollup index和base index是1:1 的, base index 上报了行数, 但rollup没有上报, 这时可以用base index的行数代替rollup的行数. 已经fix
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6313984452
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4336013846
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1276,6 +1276,45 @@ public TableStatsMeta findTableStatsStatus(long tblId) {
return idToTblStats.get(tblId);
}
+ /**
+ * The record of the table, created in the state of a table whose row count has never been collected when
+ * it doesn't exist yet. A truncation, a whole table DROP STATS and a schema change remove the record, so
+ * the rows which are loaded afterwards can only be turned into the row count of the table while the
+ * record exists: the planner falls back to it exactly while the backends have not reported the row count
+ * of the new tablets yet, which is the interval right after those operations.
+ *
+ * <p>Loads run this from the transaction visibility of the load, both on the master
+ * ({@code DatabaseTransactionMgr.updateCatalogAfterVisible}) and while a follower replays the transaction
+ * state, so the record exists on every frontend without a journal entry of its own.
+ *
+ * @return the existing or the created record, or null when the table cannot be resolved any more
+ */
+ private TableStatsMeta findOrCreateTableStats(InternalCatalog catalog, long dbId, long tableId) {
+ TableStatsMeta tableStats = idToTblStats.get(tableId);
+ if (tableStats != null) {
+ return tableStats;
+ }
+ Optional<Database> dbOption = catalog.getDb(dbId);
+ if (!dbOption.isPresent()) {
+ return null;
+ }
+ Optional<Table> tableOption = dbOption.get().getTable(tableId);
+ if (!tableOption.isPresent() || !(tableOption.get() instanceof OlapTable)) {
+ return null;
+ }
+ return findOrCreateTableStats((OlapTable) tableOption.get());
+ }
+
+ private TableStatsMeta findOrCreateTableStats(OlapTable table) {
+ TableStatsMeta tableStats = idToTblStats.get(table.getId());
+ if (tableStats != null) {
+ return tableStats;
+ }
+ TableStatsMeta created = new TableStatsMeta(table);
+ TableStatsMeta existing = idToTblStats.putIfAbsent(table.getId(), created);
Review Comment:
[P2] Preserve the stats-map lock when inserting load records
`writeTableStats()` holds the `idToTblStats` monitor while serializing its size and entries, but this insertion bypasses that monitor. During a live metadata dump (`MetaService.dump()` -> serving `Env.dumpImage()`), a cloud BE's `reportCommitTxnResult()` can reach this helper without a table lock, despite the dump's table read locks. If it inserts a first-load record after the size is written but before the iterator is created, the dump declares N records and emits N+1. `readIdToTblStats()` then leaves the extra JSON in the stream and the next module reader consumes it, making the image unreadable. This concerns live dumps; ordinary checkpoints use an isolated Env. Use the same map monitor as the existing publication/removal helpers, or serialize a consistent snapshot.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -268,6 +382,29 @@ public long getBaseIndexDeltaRowCount(OlapTable table) {
return updatedRows.get() - maxUpdateRows;
}
+ /**
+ * The row count of the index together with the rows loaded since it was collected, i.e. the row count of
+ * the table. Both are read by the planner without the table lock, for instance while it plans a direct
+ * scan of a materialized view, so they have to come from the same state of this record: a collected row
+ * count paired with the baseline of another analysis would count rows twice or miss them. The rows loaded
+ * while this call runs are not part of the snapshot, whichever state reads them accumulates them in
+ * {@link #updatedRows} and reports them as delta rows.
+ */
+ public synchronized long getRowCountWithDeltaRows(OlapTable table, long indexId) {
+ long rowCount = getRowCount(indexId);
+ if (!keepsOneRowPerBaseRow(table, indexId)) {
+ // The index aggregates, or it merges the rows of a unique key table: it has its own, smaller row
+ // count, and the rows loaded into the base index would overstate it.
+ return rowCount;
+ }
+ if (rowCount == TableIf.UNKNOWN_ROW_COUNT) {
Review Comment:
[P2] Keep a usable estimate for a newly added projection rollup
On a DUP table `t(k1,k2,v)`, load 5 rows and ANALYZE, then add `r_dup(k1,v)` and load 100 more rows before the new rollup tablets report their counts. ADD ROLLUP retains the existing `TableStatsMeta`, so it has a collected base count and delta=100 but no `r_dup` entry. The plan `Project(k1,v) -> OlapScan(t INDEX r_dup)` passes `keepsOneRowPerBaseRow()`, yet this branch returns -1 and `computeOlapScan()` clamps it to 1 although the rollup holds 105 rows. Previously the fallback returned -1+100=99. For a row-preserving index without its own snapshot, use the known base count and delta, or preserve the usable load estimate. Add coverage for ANALYZE -> ADD ROLLUP -> LOAD before the first new-index report. This needs neither injected stats nor DROP STATS.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1366,52 +1405,52 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
// Record : TableId -> (TabletId -> update rows)
if (event.getTabletRecords() != null) {
for (Entry<Long, Map<Long, Long>> record : event.getTabletRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
- if (statsStatus != null) {
- Optional<Database> dbOption = catalog.getDb(event.getDbId());
- if (!dbOption.isPresent()) {
- LOG.warn("Database {} does not exist.", event.getDbId());
- continue;
- }
- Database database = dbOption.get();
- Optional<Table> tableOption = database.getTable(record.getKey());
- if (!tableOption.isPresent()) {
- LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
- continue;
- }
- Table table = tableOption.get();
- if (!(table instanceof OlapTable)) {
- continue;
- }
- OlapTable olapTable = (OlapTable) table;
- short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
- Map<Long, Long> tabletRows = record.getValue();
- if (tabletRows == null || tabletRows.isEmpty()) {
- LOG.info("Tablet row count map is empty");
- continue;
- }
- long rowsForAllReplica = 0;
- for (Entry<Long, Long> entry : tabletRows.entrySet()) {
- rowsForAllReplica += entry.getValue();
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table id {}, tablet id {}, row count {}",
- record.getKey(), entry.getKey(), entry.getValue());
- }
- }
- long tableUpdateRows = rowsForAllReplica / replicaNum;
- LOG.info("Update rows for table {} is {}, replicaNum is {}, "
- + "rows for all replica {}, tablets count {}",
- olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
- statsStatus.updatedRows.addAndGet(tableUpdateRows);
- if (StatisticsUtil.enablePartitionAnalyze()) {
- updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ Optional<Database> dbOption = catalog.getDb(event.getDbId());
+ if (!dbOption.isPresent()) {
+ LOG.warn("Database {} does not exist.", event.getDbId());
+ continue;
+ }
+ Database database = dbOption.get();
+ Optional<Table> tableOption = database.getTable(record.getKey());
+ if (!tableOption.isPresent()) {
+ LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
+ continue;
+ }
+ Table table = tableOption.get();
+ if (!(table instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ TableStatsMeta statsStatus = findOrCreateTableStats(olapTable);
Review Comment:
[P2] Order record recreation with DROP STATS journaling
The `VISIBLE` transaction is durable before `DatabaseTransactionMgr.finishTransaction()` calls `updateCatalogAfterVisible()`. A concurrent whole-table `DROP STATS t` takes no target-table lock, so it can remove the record, journal `DELETE_TABLE_STATS`, and finish remote invalidation in that gap. Resuming here creates a record containing the load's N rows on the master. Followers and the checkpoint instead replay `VISIBLE` (create N) followed by `DELETE_TABLE_STATS` (remove it), leaving no record. Before tablet reporting, `Project(*) -> OlapScan(t)` consequently estimates N on the master but 1 after replay or failover. The previous get-only path could not resurrect the deleted record. Order creation/removal consistently with the transaction and deletion journals, and cover this interleaving with deterministic barriers.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6363443671
# FE Regression Coverage Report
Increment line coverage ` 28.57% (128/448)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_4c792cff11ff0151e7af0174bf908dcb0d8a056c_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_4c792cff11ff0151e7af0174bf908dcb0d8a056c_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6321119929
# FE UT Coverage Report
Increment line coverage ` 67.52% (79/117)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_9de2254950e70969946a99e8ce2b9a69e1d9bf6f/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_9de2254950e70969946a99e8ce2b9a69e1d9bf6f/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4345864730
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1298,12 +1344,12 @@ public void updateUpdatedRows(Map<Long, Long> partitionToUpdateRows, long dbId,
}
// Invoke this for cloud version load.
- public void updateUpdatedRows(Map<Long, Long> updatedRows) {
+ public void updateUpdatedRows(Map<Long, Long> updatedRows, long dbId) {
try {
if (!Env.getCurrentEnv().isMaster() || Env.isCheckpointThread()) {
return;
}
- UpdateRowsEvent updateRowsEvent = new UpdateRowsEvent(updatedRows);
+ UpdateRowsEvent updateRowsEvent = new UpdateRowsEvent(dbId, updatedRows);
Review Comment:
[P2] Serialize cloud row updates with statistics deletion
The new DROP STATS table lock does not protect the cloud path: `reportCommitTxnResult() -> afterCommitTxnResp() -> updateUpdatedRows()` takes no target-table lock. After a cloud report creates a record containing N rows, pause between `replayUpdateRowsRecord()` and `logUpdateRowsRecord()`, then complete whole-table DROP STATS. The master removes the record and journals DELETE; resuming the report journals UPDATE_ROWS. Replay therefore executes DELETE -> UPDATE_ROWS and recreates N, while the master has no record. Before tablet reporting, `Project(*) -> OlapScan(t)` estimates 1 on the master versus N after failover. This is distinct from the existing non-cloud VISIBLE/DROP STATS thread: it uses the separate cloud journal and the opposite replay order. Serialize cloud mutation through journal completion with the deletion protocol, and cover this interleaving with barriers.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1355,7 +1401,7 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
InternalCatalog catalog = Env.getCurrentInternalCatalog();
if (event.getRecords() != null) {
for (Entry<Long, Long> record : event.getRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
+ TableStatsMeta statsStatus = findOrCreateTableStats(catalog, event.getDbId(), record.getKey());
Review Comment:
[P2] Exclude pre-truncate cloud reports from the recreated record
Cloud stream loads queue `send_stats_to_fe_async()` and can return success before the FE receives their statistics. Load 100 rows, delay that report, TRUNCATE the entire table, then deliver the old response and load 3 new rows. TRUNCATE keeps the table ID, so this branch recreates a zero-baseline record and accumulates the removed 100 rows; the next load raises it to 103. `afterCommitTxnResp()` applies these counters before `updateVersion()` skips the now-missing old partition. While the replacement tablets are unreported, `Project(*) -> OlapScan(t)` consequently estimates 103 for 3 rows. Previously the delayed report found no record and was ignored. This is separate from the known partial-truncate/drop-partition counter issue. Validate the report's partition generation before recreating/accumulating the record, and add a delayed-report/full-truncate/reload case.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1366,52 +1412,52 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
// Record : TableId -> (TabletId -> update rows)
if (event.getTabletRecords() != null) {
for (Entry<Long, Map<Long, Long>> record : event.getTabletRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
- if (statsStatus != null) {
- Optional<Database> dbOption = catalog.getDb(event.getDbId());
- if (!dbOption.isPresent()) {
- LOG.warn("Database {} does not exist.", event.getDbId());
- continue;
- }
- Database database = dbOption.get();
- Optional<Table> tableOption = database.getTable(record.getKey());
- if (!tableOption.isPresent()) {
- LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
- continue;
- }
- Table table = tableOption.get();
- if (!(table instanceof OlapTable)) {
- continue;
- }
- OlapTable olapTable = (OlapTable) table;
- short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
- Map<Long, Long> tabletRows = record.getValue();
- if (tabletRows == null || tabletRows.isEmpty()) {
- LOG.info("Tablet row count map is empty");
- continue;
- }
- long rowsForAllReplica = 0;
- for (Entry<Long, Long> entry : tabletRows.entrySet()) {
- rowsForAllReplica += entry.getValue();
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table id {}, tablet id {}, row count {}",
- record.getKey(), entry.getKey(), entry.getValue());
- }
- }
- long tableUpdateRows = rowsForAllReplica / replicaNum;
- LOG.info("Update rows for table {} is {}, replicaNum is {}, "
- + "rows for all replica {}, tablets count {}",
- olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
- statsStatus.updatedRows.addAndGet(tableUpdateRows);
- if (StatisticsUtil.enablePartitionAnalyze()) {
- updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ Optional<Database> dbOption = catalog.getDb(event.getDbId());
+ if (!dbOption.isPresent()) {
+ LOG.warn("Database {} does not exist.", event.getDbId());
+ continue;
+ }
+ Database database = dbOption.get();
+ Optional<Table> tableOption = database.getTable(record.getKey());
+ if (!tableOption.isPresent()) {
+ LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
+ continue;
+ }
+ Table table = tableOption.get();
+ if (!(table instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ TableStatsMeta statsStatus = findOrCreateTableStats(olapTable);
Review Comment:
[P2] Order schema-completion removal with load record recreation
`SchemaChangeJobV2.runRunningJob()` journals FINISHED under the table write lock, unlocks at line 732, and removes table statistics only afterwards at line 738. A ready load can become VISIBLE in that gap and update the old record, which the schema worker then removes on the master. Replay applies FINISHED and removes statistics in `EditLog`, then replays VISIBLE; this new call recreates a record containing the load's N rows. The master thus has no record while replay/checkpoint has N. With the new index still unreported, `Project(*) -> OlapScan(t)` estimates 1 versus N after failover. The previous get-only path left both records absent. This is a distinct schema-completion caller, untouched by the DROP STATS locking fix. Remove statistics within the schema FINISHED/table-lock boundary and add a deterministic schema-completion/load interleaving test.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4266628223
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -194,6 +273,21 @@ public void update(AnalysisInfo analyzedJob, TableIf tableIf) {
if (tableIf != null) {
if (tableIf instanceof OlapTable) {
OlapTable olapTable = (OlapTable) tableIf;
+ // The collected row counts published below already include the rows which had been loaded when
+ // the job was built, remember how many they were, they are not delta rows. The baseline may
+ // only advance together with the collected base index row count, an analysis of another index
+ // (a materialized view) doesn't touch it.
+ // NOTICE: the baseline is published before the collected counts. They are both read without
+ // the table lock by the planner, and a reader which pairs the collected count of this analysis
+ // with the baseline of the previous one counts the rows loaded since that previous analysis
+ // twice.
+ if (analyzedJob.indexesRowCount.containsKey(olapTable.getBaseIndexId())) {
+ // A row count supplied by the user replaces the collected one and describes the table as
+ // of now, so the rows loaded so far are part of it and the baseline has to move to the
+ // current value. Keeping the older baseline would count those rows again once DROP STATS
+ // clears userInjected.
+ updatedRowsBase.set(analyzedJob.userInject ? updatedRows.get() : analyzedJob.updateRows);
Review Comment:
[P2] Align the baseline with the snapshot that ANALYZE actually reads
This baseline is captured when the job is built, while indexesRowCount comes from SQL executed later. On a fresh or truncated table, load 100 rows, build a FULL ANALYZE job (updateRows=100), then let another 50-row load become visible before its COUNT(1) query starts. The new load-created record has updatedRows=150 and analysis collects 150, but this line installs baseline=100, so a full OlapScan before tablet reporting gets cardinality 150+(150-100)=200. Previously the first analysis had no existing record/counter, so this sequence returned the collected 150. The monitor cannot reconcile these two different data snapshots. Pair the collected count with the load baseline for the visible-version snapshot used by the analyze query and add a deterministic load-between-job-build-and-query case.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -268,6 +382,29 @@ public long getBaseIndexDeltaRowCount(OlapTable table) {
return updatedRows.get() - maxUpdateRows;
}
+ /**
+ * The row count of the index together with the rows loaded since it was collected, i.e. the row count of
+ * the table. Both are read by the planner without the table lock, for instance while it plans a direct
+ * scan of a materialized view, so they have to come from the same state of this record: a collected row
+ * count paired with the baseline of another analysis would count rows twice or miss them. The rows loaded
+ * while this call runs are not part of the snapshot, whichever state reads them accumulates them in
+ * {@link #updatedRows} and reports them as delta rows.
+ */
+ public synchronized long getRowCountWithDeltaRows(OlapTable table, long indexId) {
+ long rowCount = getRowCount(indexId);
+ if (!keepsOneRowPerBaseRow(table, indexId)) {
+ // The index aggregates, or it merges the rows of a unique key table: it has its own, smaller row
+ // count, and the rows loaded into the base index would overstate it.
+ return rowCount;
+ }
+ if (rowCount == TableIf.UNKNOWN_ROW_COUNT) {
+ // The index has no collected row count yet, neither the row count of the table nor a delta row
+ // count can be derived from it. The planner treats the unknown row count as no information.
+ return rowCount;
+ }
+ return rowCount + getBaseIndexDeltaRowCount(table);
Review Comment:
[P2] Keep a baseline for an independently injected rollup count
A row-preserving rollup can have a current collected count while updatedRowsBase still describes the base index's older snapshot. For a fresh DUP table t(k1,k2,v) with r_dup(k1,v), load 5 rows, run ALTER TABLE t INDEX r_dup MODIFY COLUMN k1 SET STATS ('row_count'='5'), then DROP STATS t(k2). The injection updates only r_dup's count, so the base-index condition in update() leaves the new record's baseline at 0; column DROP STATS keeps that count but clears userInjected. Before tablet reporting, Project(k1,v) -> OlapScan(t, index=r_dup) therefore returns 5+(5-0)=10 here although the rollup has 5 rows. Previously rollup-only injection created a record with no accumulated rows and returned 5. Pair each index count with its own load baseline, or consistently refresh equivalent index snapshots when injecting a count, and cover this injection/drop sequence.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1366,52 +1405,52 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
// Record : TableId -> (TabletId -> update rows)
if (event.getTabletRecords() != null) {
for (Entry<Long, Map<Long, Long>> record : event.getTabletRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
- if (statsStatus != null) {
- Optional<Database> dbOption = catalog.getDb(event.getDbId());
- if (!dbOption.isPresent()) {
- LOG.warn("Database {} does not exist.", event.getDbId());
- continue;
- }
- Database database = dbOption.get();
- Optional<Table> tableOption = database.getTable(record.getKey());
- if (!tableOption.isPresent()) {
- LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
- continue;
- }
- Table table = tableOption.get();
- if (!(table instanceof OlapTable)) {
- continue;
- }
- OlapTable olapTable = (OlapTable) table;
- short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
- Map<Long, Long> tabletRows = record.getValue();
- if (tabletRows == null || tabletRows.isEmpty()) {
- LOG.info("Tablet row count map is empty");
- continue;
- }
- long rowsForAllReplica = 0;
- for (Entry<Long, Long> entry : tabletRows.entrySet()) {
- rowsForAllReplica += entry.getValue();
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table id {}, tablet id {}, row count {}",
- record.getKey(), entry.getKey(), entry.getValue());
- }
- }
- long tableUpdateRows = rowsForAllReplica / replicaNum;
- LOG.info("Update rows for table {} is {}, replicaNum is {}, "
- + "rows for all replica {}, tablets count {}",
- olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
- statsStatus.updatedRows.addAndGet(tableUpdateRows);
- if (StatisticsUtil.enablePartitionAnalyze()) {
- updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ Optional<Database> dbOption = catalog.getDb(event.getDbId());
+ if (!dbOption.isPresent()) {
+ LOG.warn("Database {} does not exist.", event.getDbId());
+ continue;
+ }
+ Database database = dbOption.get();
+ Optional<Table> tableOption = database.getTable(record.getKey());
+ if (!tableOption.isPresent()) {
+ LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
+ continue;
+ }
+ Table table = tableOption.get();
+ if (!(table instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ TableStatsMeta statsStatus = findOrCreateTableStats(olapTable);
+ short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
+ Map<Long, Long> tabletRows = record.getValue();
+ if (tabletRows == null || tabletRows.isEmpty()) {
+ LOG.info("Tablet row count map is empty");
+ continue;
+ }
+ long rowsForAllReplica = 0;
+ for (Entry<Long, Long> entry : tabletRows.entrySet()) {
+ rowsForAllReplica += entry.getValue();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Table id {}, tablet id {}, row count {}",
+ record.getKey(), entry.getKey(), entry.getValue());
}
}
+ long tableUpdateRows = rowsForAllReplica / replicaNum;
+ LOG.info("Update rows for table {} is {}, replicaNum is {}, "
+ + "rows for all replica {}, tablets count {}",
+ olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
+ statsStatus.updatedRows.addAndGet(tableUpdateRows);
Review Comment:
[P2] Exclude temporary-partition rows from the normal-table fallback
The tablet records also contain loads into temporary partitions: PublishVersionDaemon.getBaseTabletIdsForEachBe resolves their IDs through OlapTable.getPartition(long), which searches temporary partitions too. This increment counts all of them, but an ordinary OlapScan and getRowCountForIndex cover only normal partitions. On a never-analyzed range-partitioned DUP table, load 5 rows into a normal partition and 100 using INSERT INTO t TEMPORARY PARTITION(tp1) before the normal tablets' first report. The new record has collected count/baseline 0 and updatedRows=105, so Project(*) -> OlapScan(t, normal partitions) gets cardinality105 although its table contains only5 queryable rows. The later normal-only updatePartitionRows helper does not correct this global counter. Keep temporary staging rows out of the normal-table fallback, or derive that count from partition-specific deltas, and cover a temporary load alongside an unreported normal partition.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1366,52 +1405,52 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
// Record : TableId -> (TabletId -> update rows)
if (event.getTabletRecords() != null) {
for (Entry<Long, Map<Long, Long>> record : event.getTabletRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
- if (statsStatus != null) {
- Optional<Database> dbOption = catalog.getDb(event.getDbId());
- if (!dbOption.isPresent()) {
- LOG.warn("Database {} does not exist.", event.getDbId());
- continue;
- }
- Database database = dbOption.get();
- Optional<Table> tableOption = database.getTable(record.getKey());
- if (!tableOption.isPresent()) {
- LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
- continue;
- }
- Table table = tableOption.get();
- if (!(table instanceof OlapTable)) {
- continue;
- }
- OlapTable olapTable = (OlapTable) table;
- short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
- Map<Long, Long> tabletRows = record.getValue();
- if (tabletRows == null || tabletRows.isEmpty()) {
- LOG.info("Tablet row count map is empty");
- continue;
- }
- long rowsForAllReplica = 0;
- for (Entry<Long, Long> entry : tabletRows.entrySet()) {
- rowsForAllReplica += entry.getValue();
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table id {}, tablet id {}, row count {}",
- record.getKey(), entry.getKey(), entry.getValue());
- }
- }
- long tableUpdateRows = rowsForAllReplica / replicaNum;
- LOG.info("Update rows for table {} is {}, replicaNum is {}, "
- + "rows for all replica {}, tablets count {}",
- olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
- statsStatus.updatedRows.addAndGet(tableUpdateRows);
- if (StatisticsUtil.enablePartitionAnalyze()) {
- updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ Optional<Database> dbOption = catalog.getDb(event.getDbId());
+ if (!dbOption.isPresent()) {
+ LOG.warn("Database {} does not exist.", event.getDbId());
+ continue;
+ }
+ Database database = dbOption.get();
+ Optional<Table> tableOption = database.getTable(record.getKey());
+ if (!tableOption.isPresent()) {
+ LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
+ continue;
+ }
+ Table table = tableOption.get();
+ if (!(table instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ TableStatsMeta statsStatus = findOrCreateTableStats(olapTable);
+ short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
+ Map<Long, Long> tabletRows = record.getValue();
+ if (tabletRows == null || tabletRows.isEmpty()) {
+ LOG.info("Tablet row count map is empty");
+ continue;
+ }
+ long rowsForAllReplica = 0;
+ for (Entry<Long, Long> entry : tabletRows.entrySet()) {
+ rowsForAllReplica += entry.getValue();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Table id {}, tablet id {}, row count {}",
+ record.getKey(), entry.getKey(), entry.getValue());
}
}
+ long tableUpdateRows = rowsForAllReplica / replicaNum;
+ LOG.info("Update rows for table {} is {}, replicaNum is {}, "
+ + "rows for all replica {}, tablets count {}",
+ olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
+ statsStatus.updatedRows.addAndGet(tableUpdateRows);
+ if (StatisticsUtil.enablePartitionAnalyze()) {
+ updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ }
}
return;
}
- // Handle truncate table
+ // Handle truncate table. A partition truncation is not a load: the rows it reports are the rows it
Review Comment:
[P2] Invalidate the load-derived count when partitions are removed
A partition truncation also reaches records created by a load, but the retained branch below adds the positive number of removed rows to updatedRows. For a never-analyzed DUP table with two reported 50-row partitions, the new record has a zero collected count/baseline and updatedRows=100. Truncating one partition and inserting 5 rows into its replacement makes this counter 155 although the table has 55 rows. The first load marks the replacement index's row count unreported, so an unpruned scan takes getRowCountWithDeltaRows and uses 155 as cardinality. DROP PARTITION similarly retains the historical count: dropping one 50-row partition, adding a replacement, and loading 5 yields 105 instead of 55 through the same fallback. Before this change an unanalyzed table had no record to turn these modification counts into cardinality. Invalidate or adjust the load-derived count when partitions are removed, including partial TRUNCATE and DROP PARTITION, and cover removal followed by load.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6258752115
run nonConcurrent
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6258749919
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](fe) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6145645792
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://doris.apache.org/community/how-to-contribute/review-and-ci).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6160256613
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6161210502
<details>
<summary>TPC-DS: <b>Total hot run time: 152567 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit e2ce99b43fe1332fbad22f00c5019273a29bf94c, data reload: false
query5 4322 585 459 459
query6 426 218 193 193
query7 4799 451 223 223
query8 321 188 169 169
query9 8749 3937 3973 3937
query10 461 310 259 259
query11 5899 3454 3175 3175
query12 149 89 84 84
query13 1273 460 335 335
query14 6552 4751 4472 4472
query14_1 4290 4284 4184 4184
query15 214 216 188 188
query16 951 450 430 430
query17 866 679 538 538
query18 2420 427 316 316
query19 205 167 139 139
query20 85 84 81 81
query21 213 135 117 117
query22 13084 13104 12879 12879
query23 13339 12525 12122 12122
query23_1 12250 12097 12097 12097
query24 7117 823 463 463
query24_1 478 484 471 471
query25 543 425 370 370
query26 1306 261 142 142
query27 2766 452 274 274
query28 4586 1940 1949 1940
query29 1576 606 472 472
query30 304 222 193 193
query31 892 777 665 665
query32 140 100 91 91
query33 510 314 253 253
query34 880 871 502 502
query35 755 782 657 657
query36 852 816 736 736
query37 138 102 101 101
query38 1794 1756 1701 1701
query39 735 734 703 703
query39_1 687 694 691 691
query40 224 120 100 100
query41 72 72 70 70
query42 90 85 84 84
query43 373 377 332 332
query44 1320 707 726 707
query45 184 194 164 164
query46 845 967 585 585
query47 2939 2926 2965 2926
query48 306 316 226 226
query49 609 398 306 306
query50 689 274 213 213
query51 10770 10582 10617 10582
query52 82 79 69 69
query53 187 206 154 154
query54 232 191 172 172
query55 72 67 62 62
query56 226 213 205 205
query57 1613 1575 1566 1566
query58 283 260 250 250
query59 2347 2383 2110 2110
query60 280 236 216 216
query61 149 142 140 140
query62 411 349 286 286
query63 190 165 162 162
query64 2738 982 803 803
query65 3438 3398 3405 3398
query66 1802 417 296 296
query67 20216 20165 20091 20091
query68 2996 1012 619 619
query69 380 283 284 283
query70 919 830 814 814
query71 284 235 210 210
query72 2702 2550 2237 2237
query73 510 528 303 303
query74 4628 4489 4320 4320
query75 2349 2321 1960 1960
query76 2293 1008 595 595
query77 367 398 301 301
query78 9082 8961 8449 8449
query79 965 833 513 513
query80 1036 444 350 350
query81 577 325 278 278
query82 581 135 108 108
query83 292 201 181 181
query84 310 122 95 95
query85 854 436 364 364
query86 383 239 230 230
query87 2028 1959 1862 1862
query88 3662 2729 2677 2677
query89 338 301 268 268
query90 1847 182 183 182
query91 160 152 123 123
query92 90 87 87 87
query93 1005 984 571 571
query94 580 317 285 285
query95 586 364 383 364
query96 662 530 244 244
query97 2441 2485 2357 2357
query98 159 147 147 147
query99 799 793 656 656
Total cold run time: 235196 ms
Total hot run time: 152567 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6362941517
<details>
<summary>TPC-DS: <b>Total hot run time: 149791 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 4c792cff11ff0151e7af0174bf908dcb0d8a056c, data reload: false
query5 4331 584 442 442
query6 429 205 193 193
query7 4796 440 245 245
query8 328 165 146 146
query9 8778 3068 3070 3068
query10 491 311 259 259
query11 5944 3818 3564 3564
query12 147 95 84 84
query13 1251 423 316 316
query14 6102 3917 3617 3617
query14_1 3398 3416 3403 3403
query15 205 196 184 184
query16 943 464 422 422
query17 858 658 525 525
query18 2430 429 311 311
query19 199 166 138 138
query20 85 82 79 79
query21 220 124 106 106
query22 12883 12878 12803 12803
query23 13595 12943 12357 12357
query23_1 12692 12638 12474 12474
query24 7356 803 467 467
query24_1 470 480 476 476
query25 531 423 362 362
query26 1267 255 141 141
query27 2781 440 271 271
query28 4604 1966 1945 1945
query29 1561 601 463 463
query30 330 218 184 184
query31 1003 928 816 816
query32 197 93 91 91
query33 536 327 229 229
query34 878 848 499 499
query35 733 740 652 652
query36 818 822 714 714
query37 118 97 79 79
query38 1759 1769 1642 1642
query39 795 781 728 728
query39_1 728 732 736 732
query40 219 110 91 91
query41 67 61 60 60
query42 83 79 81 79
query43 334 354 304 304
query44 1298 677 702 677
query45 175 190 164 164
query46 813 956 582 582
query47 2725 3005 2730 2730
query48 288 294 216 216
query49 586 396 291 291
query50 656 268 211 211
query51 10338 10258 10282 10258
query52 88 84 74 74
query53 181 214 146 146
query54 245 197 170 170
query55 70 64 64 64
query56 257 224 188 188
query57 1665 1606 1577 1577
query58 294 260 256 256
query59 2124 2163 1919 1919
query60 288 239 224 224
query61 144 142 141 141
query62 387 331 286 286
query63 184 152 157 152
query64 2733 906 717 717
query65 3314 3245 3254 3245
query66 1763 414 312 312
query67 19999 20158 19770 19770
query68 2945 985 602 602
query69 391 287 238 238
query70 816 779 766 766
query71 298 233 207 207
query72 2712 2578 2331 2331
query73 515 525 289 289
query74 4620 4522 4361 4361
query75 2338 2307 1902 1902
query76 2360 996 566 566
query77 352 365 261 261
query78 9926 9707 9123 9123
query79 907 900 520 520
query80 534 410 355 355
query81 534 322 274 274
query82 863 129 103 103
query83 312 189 176 176
query84 322 122 101 101
query85 825 412 362 362
query86 316 238 232 232
query87 1959 1954 1804 1804
query88 3705 2652 2652 2652
query89 325 280 237 237
query90 1869 162 165 162
query91 162 152 128 128
query92 98 77 79 77
query93 1020 984 564 564
query94 442 271 268 268
query95 577 324 352 324
query96 651 484 229 229
query97 2485 2427 2335 2335
query98 157 150 147 147
query99 780 756 647 647
Total cold run time: 233702 ms
Total hot run time: 149791 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6319883205
<details>
<summary>TPC-DS: <b>Total hot run time: 150505 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 9de2254950e70969946a99e8ce2b9a69e1d9bf6f, data reload: false
query5 4335 577 446 446
query6 419 202 198 198
query7 4791 441 222 222
query8 324 161 141 141
query9 8775 3062 3056 3056
query10 489 305 246 246
query11 5995 3857 3573 3573
query12 159 101 86 86
query13 1246 443 316 316
query14 6108 3905 3659 3659
query14_1 3456 3432 3438 3432
query15 208 190 184 184
query16 951 416 402 402
query17 847 664 533 533
query18 2426 431 309 309
query19 210 165 136 136
query20 85 98 85 85
query21 210 129 106 106
query22 12984 13128 12765 12765
query23 13457 12787 12523 12523
query23_1 12532 12701 12667 12667
query24 7403 804 464 464
query24_1 471 477 474 474
query25 541 424 355 355
query26 1260 261 142 142
query27 2766 432 270 270
query28 4653 1921 1937 1921
query29 1575 595 446 446
query30 294 214 189 189
query31 1006 924 818 818
query32 142 91 93 91
query33 526 300 247 247
query34 926 844 490 490
query35 733 785 638 638
query36 822 875 715 715
query37 134 100 84 84
query38 1771 1729 1649 1649
query39 800 809 747 747
query39_1 738 721 741 721
query40 222 116 99 99
query41 72 70 67 67
query42 90 89 82 82
query43 345 366 310 310
query44 1293 687 700 687
query45 181 173 163 163
query46 826 964 567 567
query47 3014 2783 2950 2783
query48 309 309 219 219
query49 577 405 294 294
query50 687 318 202 202
query51 10267 10439 10220 10220
query52 82 79 68 68
query53 182 205 149 149
query54 242 184 181 181
query55 74 72 61 61
query56 239 214 231 214
query57 1658 1628 1572 1572
query58 283 264 262 262
query59 2135 2218 1999 1999
query60 302 250 224 224
query61 143 148 141 141
query62 384 339 295 295
query63 192 152 155 152
query64 3361 919 831 831
query65 3318 3275 3248 3248
query66 1782 424 305 305
query67 20117 19857 19807 19807
query68 2999 983 592 592
query69 380 313 247 247
query70 858 775 740 740
query71 299 234 215 215
query72 2671 2318 2298 2298
query73 511 512 293 293
query74 4582 4519 4370 4370
query75 2352 2286 1908 1908
query76 2317 1007 573 573
query77 344 384 282 282
query78 9861 9752 9164 9164
query79 1001 830 502 502
query80 1252 424 351 351
query81 583 316 268 268
query82 612 135 108 108
query83 307 192 173 173
query84 310 117 102 102
query85 876 425 372 372
query86 389 240 231 231
query87 2024 1925 1788 1788
query88 3598 2640 2631 2631
query89 321 272 244 244
query90 1933 167 163 163
query91 166 151 132 132
query92 96 82 86 82
query93 1030 972 566 566
query94 641 314 260 260
query95 575 329 366 329
query96 658 470 230 230
query97 2450 2420 2319 2319
query98 161 177 147 147
query99 763 765 652 652
Total cold run time: 235626 ms
Total hot run time: 150505 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6323057167
# FE Regression Coverage Report
Increment line coverage ` 74.36% (87/117)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_9de2254950e70969946a99e8ce2b9a69e1d9bf6f_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_9de2254950e70969946a99e8ce2b9a69e1d9bf6f_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4312279362
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -194,6 +273,21 @@ public void update(AnalysisInfo analyzedJob, TableIf tableIf) {
if (tableIf != null) {
if (tableIf instanceof OlapTable) {
OlapTable olapTable = (OlapTable) tableIf;
+ // The collected row counts published below already include the rows which had been loaded when
+ // the job was built, remember how many they were, they are not delta rows. The baseline may
+ // only advance together with the collected base index row count, an analysis of another index
+ // (a materialized view) doesn't touch it.
+ // NOTICE: the baseline is published before the collected counts. They are both read without
+ // the table lock by the planner, and a reader which pairs the collected count of this analysis
+ // with the baseline of the previous one counts the rows loaded since that previous analysis
+ // twice.
+ if (analyzedJob.indexesRowCount.containsKey(olapTable.getBaseIndexId())) {
+ // A row count supplied by the user replaces the collected one and describes the table as
+ // of now, so the rows loaded so far are part of it and the baseline has to move to the
+ // current value. Keeping the older baseline would count those rows again once DROP STATS
+ // clears userInjected.
+ updatedRowsBase.set(analyzedJob.userInject ? updatedRows.get() : analyzedJob.updateRows);
Review Comment:
这是老代码固有问题, 设计使用deltaRow时就已知有这个缺陷. 提了新issue 跟踪 https://github.com/apache/doris/issues/69280
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1366,52 +1405,52 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
// Record : TableId -> (TabletId -> update rows)
if (event.getTabletRecords() != null) {
for (Entry<Long, Map<Long, Long>> record : event.getTabletRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
- if (statsStatus != null) {
- Optional<Database> dbOption = catalog.getDb(event.getDbId());
- if (!dbOption.isPresent()) {
- LOG.warn("Database {} does not exist.", event.getDbId());
- continue;
- }
- Database database = dbOption.get();
- Optional<Table> tableOption = database.getTable(record.getKey());
- if (!tableOption.isPresent()) {
- LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
- continue;
- }
- Table table = tableOption.get();
- if (!(table instanceof OlapTable)) {
- continue;
- }
- OlapTable olapTable = (OlapTable) table;
- short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
- Map<Long, Long> tabletRows = record.getValue();
- if (tabletRows == null || tabletRows.isEmpty()) {
- LOG.info("Tablet row count map is empty");
- continue;
- }
- long rowsForAllReplica = 0;
- for (Entry<Long, Long> entry : tabletRows.entrySet()) {
- rowsForAllReplica += entry.getValue();
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table id {}, tablet id {}, row count {}",
- record.getKey(), entry.getKey(), entry.getValue());
- }
- }
- long tableUpdateRows = rowsForAllReplica / replicaNum;
- LOG.info("Update rows for table {} is {}, replicaNum is {}, "
- + "rows for all replica {}, tablets count {}",
- olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
- statsStatus.updatedRows.addAndGet(tableUpdateRows);
- if (StatisticsUtil.enablePartitionAnalyze()) {
- updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ Optional<Database> dbOption = catalog.getDb(event.getDbId());
+ if (!dbOption.isPresent()) {
+ LOG.warn("Database {} does not exist.", event.getDbId());
+ continue;
+ }
+ Database database = dbOption.get();
+ Optional<Table> tableOption = database.getTable(record.getKey());
+ if (!tableOption.isPresent()) {
+ LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
+ continue;
+ }
+ Table table = tableOption.get();
+ if (!(table instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ TableStatsMeta statsStatus = findOrCreateTableStats(olapTable);
+ short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
+ Map<Long, Long> tabletRows = record.getValue();
+ if (tabletRows == null || tabletRows.isEmpty()) {
+ LOG.info("Tablet row count map is empty");
+ continue;
+ }
+ long rowsForAllReplica = 0;
+ for (Entry<Long, Long> entry : tabletRows.entrySet()) {
+ rowsForAllReplica += entry.getValue();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Table id {}, tablet id {}, row count {}",
+ record.getKey(), entry.getKey(), entry.getValue());
}
}
+ long tableUpdateRows = rowsForAllReplica / replicaNum;
+ LOG.info("Update rows for table {} is {}, replicaNum is {}, "
+ + "rows for all replica {}, tablets count {}",
+ olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
+ statsStatus.updatedRows.addAndGet(tableUpdateRows);
+ if (StatisticsUtil.enablePartitionAnalyze()) {
+ updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ }
}
return;
}
- // Handle truncate table
+ // Handle truncate table. A partition truncation is not a load: the rows it reports are the rows it
Review Comment:
这是老代码固有问题, 设计使用deltaRow时就已知有这个缺陷. 提了新issue 跟踪 https://github.com/apache/doris/issues/69280
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6161270848
<details>
<summary>ClickBench: <b>Total hot run time: 24.62 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit e2ce99b43fe1332fbad22f00c5019273a29bf94c, data reload: false
query1 0.01 0.01 0.01
query2 0.10 0.06 0.05
query3 0.28 0.12 0.11
query4 1.61 0.12 0.12
query5 0.28 0.25 0.25
query6 1.21 0.68 0.69
query7 0.04 0.01 0.01
query8 0.05 0.04 0.04
query9 0.43 0.35 0.36
query10 0.58 0.58 0.58
query11 0.20 0.14 0.13
query12 0.18 0.14 0.15
query13 0.50 0.50 0.51
query14 0.98 0.98 0.98
query15 0.64 0.60 0.63
query16 0.36 0.36 0.35
query17 1.14 1.14 1.10
query18 0.22 0.21 0.22
query19 2.12 1.92 1.96
query20 0.02 0.01 0.02
query21 15.49 0.27 0.14
query22 4.53 0.06 0.05
query23 16.25 0.31 0.12
query24 3.40 0.58 0.55
query25 0.10 0.05 0.05
query26 1.36 0.24 0.15
query27 0.04 0.04 0.03
query28 2.34 0.61 0.41
query29 12.50 4.18 3.29
query30 0.28 0.14 0.14
query31 2.77 0.57 0.35
query32 3.23 0.62 0.52
query33 3.39 3.18 3.25
query34 15.52 4.00 3.38
query35 3.35 3.34 3.34
query36 0.57 0.43 0.46
query37 0.10 0.06 0.07
query38 0.05 0.04 0.03
query39 0.04 0.03 0.03
query40 0.17 0.14 0.14
query41 0.08 0.03 0.03
query42 0.04 0.03 0.03
query43 0.04 0.03 0.04
Total cold run time: 96.59 s
Total hot run time: 24.62 s
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6163640709
# FE UT Coverage Report
Increment line coverage ` 65.55% (78/119)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_e2ce99b43fe1332fbad22f00c5019273a29bf94c/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_e2ce99b43fe1332fbad22f00c5019273a29bf94c/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6343595782
# FE UT Coverage Report
Increment line coverage ` 71.21% (94/132)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_70184f06f222076c5ba0b094186ccf27d09fc006/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_70184f06f222076c5ba0b094186ccf27d09fc006/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6335201465
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6354755601
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6313983451
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6342191781
<details>
<summary>TPC-DS: <b>Total hot run time: 150642 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 70184f06f222076c5ba0b094186ccf27d09fc006, data reload: false
query5 4328 600 449 449
query6 416 211 194 194
query7 4812 443 223 223
query8 336 158 146 146
query9 8787 3119 3102 3102
query10 478 311 253 253
query11 5807 3830 3586 3586
query12 143 95 86 86
query13 1260 434 328 328
query14 6058 3924 3621 3621
query14_1 3445 3389 3479 3389
query15 211 210 182 182
query16 957 474 420 420
query17 869 672 546 546
query18 2438 426 316 316
query19 196 166 137 137
query20 86 86 91 86
query21 212 127 107 107
query22 12903 12946 12830 12830
query23 13590 12870 12429 12429
query23_1 12533 12502 12458 12458
query24 7351 837 474 474
query24_1 472 485 482 482
query25 582 417 371 371
query26 1273 262 136 136
query27 2787 449 277 277
query28 4602 1970 1971 1970
query29 1577 614 461 461
query30 305 225 185 185
query31 1015 935 824 824
query32 155 96 95 95
query33 523 336 236 236
query34 894 846 482 482
query35 725 761 655 655
query36 810 803 715 715
query37 121 97 80 80
query38 1778 1706 1648 1648
query39 800 793 746 746
query39_1 738 729 754 729
query40 218 119 96 96
query41 66 62 61 61
query42 86 83 79 79
query43 345 353 306 306
query44 1314 680 691 680
query45 176 169 167 167
query46 848 957 581 581
query47 2865 2877 2937 2877
query48 290 308 221 221
query49 571 410 303 303
query50 683 272 200 200
query51 10468 10280 10457 10280
query52 80 80 73 73
query53 185 220 154 154
query54 241 189 176 176
query55 75 72 68 68
query56 241 217 219 217
query57 1714 1625 1561 1561
query58 288 269 301 269
query59 2068 2177 1895 1895
query60 288 260 230 230
query61 149 148 149 148
query62 384 331 309 309
query63 188 155 158 155
query64 2725 968 794 794
query65 3323 3286 3317 3286
query66 1782 419 297 297
query67 20175 20050 19890 19890
query68 3268 1014 607 607
query69 404 292 250 250
query70 840 780 761 761
query71 294 238 207 207
query72 2823 2631 2265 2265
query73 525 531 294 294
query74 4651 4534 4359 4359
query75 2352 2281 1910 1910
query76 2412 1023 583 583
query77 343 385 275 275
query78 9598 9752 9154 9154
query79 1015 854 525 525
query80 564 424 342 342
query81 548 314 270 270
query82 821 135 106 106
query83 305 206 190 190
query84 328 128 108 108
query85 945 515 380 380
query86 338 241 215 215
query87 2033 1979 1780 1780
query88 3670 2712 2681 2681
query89 336 278 244 244
query90 1848 170 169 169
query91 166 151 129 129
query92 106 85 93 85
query93 1060 1013 591 591
query94 448 303 258 258
query95 588 329 351 329
query96 669 478 235 235
query97 2469 2428 2308 2308
query98 162 153 147 147
query99 768 751 666 666
Total cold run time: 234552 ms
Total hot run time: 150642 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4356778779
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/transaction/CloudGlobalTransactionMgr.java:
##########
@@ -545,7 +554,7 @@ public void afterCommitTxnResp(CommitTxnResponse commitTxnResponse, List<TabletC
}
}
Env env = Env.getCurrentEnv();
- env.getAnalysisManager().updateUpdatedRows(updatedRows);
+ env.getAnalysisManager().updateUpdatedRows(updatedRows, dbId, reportedPartitions);
Review Comment:
不修. 这是cloud模式在原代码上已有的问题. 这个pr 修复了3种commit 里最常见的1种. 对另外两种没有改进也没有损坏. 没有处理的两种commit:
1. FE 显式事务（BEGIN; INSERT ...; COMMIT;
2. BE 直连提交（enable_stream_load_commit_txn_on_be=true 的非 MoW/non-TSO stream load )
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "yujun777 (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6319685936
<details>
<summary>TPC-H: <b>Total hot run time: 28634 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 9de2254950e70969946a99e8ce2b9a69e1d9bf6f, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17589 5137 5126 5126
q2 2117 302 257 257
q3 10251 1357 818 818
q4 4681 448 332 332
q5 7413 787 511 511
q6 177 185 150 150
q7 725 784 585 585
q8 9246 1273 1068 1068
q9 5399 4168 4158 4158
q10 6824 1281 1012 1012
q11 413 250 237 237
q12 638 394 279 279
q13 18106 3532 2946 2946
q14 274 267 237 237
q15 q16 715 707 649 649
q17 1170 745 570 570
q18 6909 6162 6106 6106
q19 1097 957 568 568
q20 377 325 226 226
q21 4493 2524 2524 2524
q22 376 328 275 275
Total cold run time: 98990 ms
Total hot run time: 28634 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 5876 5830 5850 5830
q2 696 610 551 551
q3 5529 5699 5232 5232
q4 2039 2146 1559 1559
q5 5967 5672 5735 5672
q6 241 187 145 145
q7 2206 2008 1794 1794
q8 2983 2566 2559 2559
q9 7841 7533 7432 7432
q10 3634 3564 3146 3146
q11 578 404 376 376
q12 654 656 504 504
q13 3234 3555 3004 3004
q14 278 283 255 255
q15 q16 649 680 612 612
q17 7721 6997 6863 6863
q18 12989 12194 12866 12194
q19 867 762 753 753
q20 2156 2130 1934 1934
q21 5597 4575 4663 4575
q22 488 447 417 417
Total cold run time: 72223 ms
Total hot run time: 65407 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6319998889
<details>
<summary>ClickBench: <b>Total hot run time: 24.32 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 9de2254950e70969946a99e8ce2b9a69e1d9bf6f, data reload: false
query1 0.01 0.00 0.00
query2 0.10 0.05 0.05
query3 0.26 0.09 0.09
query4 1.61 0.09 0.10
query5 0.23 0.24 0.23
query6 1.20 0.69 0.69
query7 0.03 0.01 0.00
query8 0.06 0.04 0.04
query9 0.40 0.34 0.33
query10 0.61 0.58 0.61
query11 0.20 0.14 0.14
query12 0.18 0.14 0.14
query13 0.51 0.50 0.50
query14 1.02 1.03 1.00
query15 0.62 0.60 0.61
query16 0.34 0.35 0.34
query17 1.09 1.12 1.12
query18 0.19 0.19 0.19
query19 2.13 2.02 2.02
query20 0.02 0.01 0.02
query21 15.48 0.18 0.14
query22 4.92 0.06 0.05
query23 16.31 0.32 0.13
query24 3.24 0.56 0.40
query25 0.11 0.06 0.04
query26 1.44 0.22 0.16
query27 0.05 0.04 0.05
query28 2.75 0.69 0.36
query29 12.48 4.15 3.23
query30 0.29 0.13 0.10
query31 2.78 0.55 0.32
query32 3.22 0.59 0.51
query33 3.23 3.28 3.19
query34 15.41 3.98 3.41
query35 3.32 3.31 3.32
query36 0.54 0.42 0.41
query37 0.09 0.07 0.06
query38 0.05 0.04 0.03
query39 0.04 0.03 0.03
query40 0.17 0.15 0.15
query41 0.09 0.03 0.03
query42 0.04 0.04 0.03
query43 0.04 0.04 0.03
Total cold run time: 96.9 s
Total hot run time: 24.32 s
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6345986179
# FE Regression Coverage Report
Increment line coverage ` 47.39% (100/211)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_70184f06f222076c5ba0b094186ccf27d09fc006_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_70184f06f222076c5ba0b094186ccf27d09fc006_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6358947109
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6262443385
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6258748700
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6264549195
run compile
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6271372147
# FE Regression Coverage Report
Increment line coverage ` 74.36% (87/117)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69028_c56107d36aec1854e5431cd7336d253041520d9b_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69028_c56107d36aec1854e5431cd7336d253041520d9b_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4344419105
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1366,52 +1405,52 @@ public void replayUpdateRowsRecord(UpdateRowsEvent event) {
// Record : TableId -> (TabletId -> update rows)
if (event.getTabletRecords() != null) {
for (Entry<Long, Map<Long, Long>> record : event.getTabletRecords().entrySet()) {
- TableStatsMeta statsStatus = idToTblStats.get(record.getKey());
- if (statsStatus != null) {
- Optional<Database> dbOption = catalog.getDb(event.getDbId());
- if (!dbOption.isPresent()) {
- LOG.warn("Database {} does not exist.", event.getDbId());
- continue;
- }
- Database database = dbOption.get();
- Optional<Table> tableOption = database.getTable(record.getKey());
- if (!tableOption.isPresent()) {
- LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
- continue;
- }
- Table table = tableOption.get();
- if (!(table instanceof OlapTable)) {
- continue;
- }
- OlapTable olapTable = (OlapTable) table;
- short replicaNum = olapTable.getTableProperty().getReplicaAllocation().getTotalReplicaNum();
- Map<Long, Long> tabletRows = record.getValue();
- if (tabletRows == null || tabletRows.isEmpty()) {
- LOG.info("Tablet row count map is empty");
- continue;
- }
- long rowsForAllReplica = 0;
- for (Entry<Long, Long> entry : tabletRows.entrySet()) {
- rowsForAllReplica += entry.getValue();
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table id {}, tablet id {}, row count {}",
- record.getKey(), entry.getKey(), entry.getValue());
- }
- }
- long tableUpdateRows = rowsForAllReplica / replicaNum;
- LOG.info("Update rows for table {} is {}, replicaNum is {}, "
- + "rows for all replica {}, tablets count {}",
- olapTable.getName(), tableUpdateRows, replicaNum, rowsForAllReplica, tabletRows.size());
- statsStatus.updatedRows.addAndGet(tableUpdateRows);
- if (StatisticsUtil.enablePartitionAnalyze()) {
- updatePartitionRows(olapTable, tabletRows, statsStatus, replicaNum);
+ Optional<Database> dbOption = catalog.getDb(event.getDbId());
+ if (!dbOption.isPresent()) {
+ LOG.warn("Database {} does not exist.", event.getDbId());
+ continue;
+ }
+ Database database = dbOption.get();
+ Optional<Table> tableOption = database.getTable(record.getKey());
+ if (!tableOption.isPresent()) {
+ LOG.warn("Table {} does not exist in DB {}.", record.getKey(), event.getDbId());
+ continue;
+ }
+ Table table = tableOption.get();
+ if (!(table instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ TableStatsMeta statsStatus = findOrCreateTableStats(olapTable);
Review Comment:
drop stats 需要加锁
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1276,6 +1276,45 @@ public TableStatsMeta findTableStatsStatus(long tblId) {
return idToTblStats.get(tblId);
}
+ /**
+ * The record of the table, created in the state of a table whose row count has never been collected when
+ * it doesn't exist yet. A truncation, a whole table DROP STATS and a schema change remove the record, so
+ * the rows which are loaded afterwards can only be turned into the row count of the table while the
+ * record exists: the planner falls back to it exactly while the backends have not reported the row count
+ * of the new tablets yet, which is the interval right after those operations.
+ *
+ * <p>Loads run this from the transaction visibility of the load, both on the master
+ * ({@code DatabaseTransactionMgr.updateCatalogAfterVisible}) and while a follower replays the transaction
+ * state, so the record exists on every frontend without a journal entry of its own.
+ *
+ * @return the existing or the created record, or null when the table cannot be resolved any more
+ */
+ private TableStatsMeta findOrCreateTableStats(InternalCatalog catalog, long dbId, long tableId) {
+ TableStatsMeta tableStats = idToTblStats.get(tableId);
+ if (tableStats != null) {
+ return tableStats;
+ }
+ Optional<Database> dbOption = catalog.getDb(dbId);
+ if (!dbOption.isPresent()) {
+ return null;
+ }
+ Optional<Table> tableOption = dbOption.get().getTable(tableId);
+ if (!tableOption.isPresent() || !(tableOption.get() instanceof OlapTable)) {
+ return null;
+ }
+ return findOrCreateTableStats((OlapTable) tableOption.get());
+ }
+
+ private TableStatsMeta findOrCreateTableStats(OlapTable table) {
+ TableStatsMeta tableStats = idToTblStats.get(table.getId());
+ if (tableStats != null) {
+ return tableStats;
+ }
+ TableStatsMeta created = new TableStatsMeta(table);
+ TableStatsMeta existing = idToTblStats.putIfAbsent(table.getId(), created);
Review Comment:
drop stats 需要加锁
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4344413029
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1276,6 +1276,45 @@ public TableStatsMeta findTableStatsStatus(long tblId) {
return idToTblStats.get(tblId);
}
+ /**
+ * The record of the table, created in the state of a table whose row count has never been collected when
+ * it doesn't exist yet. A truncation, a whole table DROP STATS and a schema change remove the record, so
+ * the rows which are loaded afterwards can only be turned into the row count of the table while the
+ * record exists: the planner falls back to it exactly while the backends have not reported the row count
+ * of the new tablets yet, which is the interval right after those operations.
+ *
+ * <p>Loads run this from the transaction visibility of the load, both on the master
+ * ({@code DatabaseTransactionMgr.updateCatalogAfterVisible}) and while a follower replays the transaction
+ * state, so the record exists on every frontend without a journal entry of its own.
+ *
+ * @return the existing or the created record, or null when the table cannot be resolved any more
+ */
+ private TableStatsMeta findOrCreateTableStats(InternalCatalog catalog, long dbId, long tableId) {
+ TableStatsMeta tableStats = idToTblStats.get(tableId);
+ if (tableStats != null) {
+ return tableStats;
+ }
+ Optional<Database> dbOption = catalog.getDb(dbId);
+ if (!dbOption.isPresent()) {
+ return null;
+ }
+ Optional<Table> tableOption = dbOption.get().getTable(tableId);
+ if (!tableOption.isPresent() || !(tableOption.get() instanceof OlapTable)) {
+ return null;
+ }
+ return findOrCreateTableStats((OlapTable) tableOption.get());
+ }
+
+ private TableStatsMeta findOrCreateTableStats(OlapTable table) {
+ TableStatsMeta tableStats = idToTblStats.get(table.getId());
+ if (tableStats != null) {
+ return tableStats;
+ }
+ TableStatsMeta created = new TableStatsMeta(table);
+ TableStatsMeta existing = idToTblStats.putIfAbsent(table.getId(), created);
Review Comment:
drop stats 需要加锁
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4345353934
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/AnalysisManager.java:
##########
@@ -1276,6 +1276,45 @@ public TableStatsMeta findTableStatsStatus(long tblId) {
return idToTblStats.get(tblId);
}
+ /**
+ * The record of the table, created in the state of a table whose row count has never been collected when
+ * it doesn't exist yet. A truncation, a whole table DROP STATS and a schema change remove the record, so
+ * the rows which are loaded afterwards can only be turned into the row count of the table while the
+ * record exists: the planner falls back to it exactly while the backends have not reported the row count
+ * of the new tablets yet, which is the interval right after those operations.
+ *
+ * <p>Loads run this from the transaction visibility of the load, both on the master
+ * ({@code DatabaseTransactionMgr.updateCatalogAfterVisible}) and while a follower replays the transaction
+ * state, so the record exists on every frontend without a journal entry of its own.
+ *
+ * @return the existing or the created record, or null when the table cannot be resolved any more
+ */
+ private TableStatsMeta findOrCreateTableStats(InternalCatalog catalog, long dbId, long tableId) {
+ TableStatsMeta tableStats = idToTblStats.get(tableId);
+ if (tableStats != null) {
+ return tableStats;
+ }
+ Optional<Database> dbOption = catalog.getDb(dbId);
+ if (!dbOption.isPresent()) {
+ return null;
+ }
+ Optional<Table> tableOption = dbOption.get().getTable(tableId);
+ if (!tableOption.isPresent() || !(tableOption.get() instanceof OlapTable)) {
+ return null;
+ }
+ return findOrCreateTableStats((OlapTable) tableOption.get());
+ }
+
+ private TableStatsMeta findOrCreateTableStats(OlapTable table) {
+ TableStatsMeta tableStats = idToTblStats.get(table.getId());
+ if (tableStats != null) {
+ return tableStats;
+ }
+ TableStatsMeta created = new TableStatsMeta(table);
+ TableStatsMeta existing = idToTblStats.putIfAbsent(table.getId(), created);
Review Comment:
fix: 将这段代码放在 synchronized (idToTblStats) { ...} 里
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6335203324
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69028:
URL: https://github.com/apache/doris/pull/69028#issuecomment-6341984689
<details>
<summary>TPC-H: <b>Total hot run time: 28860 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 70184f06f222076c5ba0b094186ccf27d09fc006, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17613 5208 5091 5091
q2 2189 308 263 263
q3 10201 1384 817 817
q4 4684 461 345 345
q5 7430 814 530 530
q6 183 185 156 156
q7 749 808 611 611
q8 9256 1267 1072 1072
q9 5702 4206 4204 4204
q10 6832 1305 1043 1043
q11 423 249 229 229
q12 638 406 281 281
q13 18100 3672 3047 3047
q14 279 274 252 252
q15 q16 729 713 666 666
q17 1190 751 578 578
q18 7082 6143 6090 6090
q19 1115 972 600 600
q20 380 337 221 221
q21 5654 2481 2499 2481
q22 389 323 283 283
Total cold run time: 100818 ms
Total hot run time: 28860 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 6036 5935 6134 5935
q2 702 606 579 579
q3 5538 5758 5310 5310
q4 2042 2171 1586 1586
q5 5792 5829 5546 5546
q6 237 185 147 147
q7 2196 2101 1743 1743
q8 2976 2639 2571 2571
q9 7882 7888 7518 7518
q10 3654 3580 3130 3130
q11 582 411 394 394
q12 672 664 499 499
q13 3343 3671 3061 3061
q14 293 282 257 257
q15 q16 680 681 628 628
q17 7724 7029 6860 6860
q18 12945 12190 12819 12190
q19 902 817 805 805
q20 2154 2145 1902 1902
q21 5821 4664 4720 4664
q22 488 469 408 408
Total cold run time: 72659 ms
Total hot run time: 65733 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4310028634
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java:
##########
@@ -384,11 +384,19 @@ public Statistics visitLogicalFilter(LogicalFilter<? extends Plan> filter, Void
* returns the sum of deltaRowCount for all selected partitions or for the table.
*/
private long computeDeltaRowCount(OlapScan olapScan) {
+ OlapTable olapTable = olapScan.getTable();
+ // The delta rows are the rows loaded into the base index which the collected base index row count
+ // doesn't include yet. They are not the rows of an index which aggregates them: such an index has its
+ // own (usually much smaller) row count, so the delta only applies to a scan of an index which keeps
+ // one row per base row.
+ if (!TableStatsMeta.keepsOneRowPerBaseRow(olapTable, olapScan.getSelectedIndexId())) {
+ return 0;
Review Comment:
[P2] Preserve stale-range recovery for non-row-preserving indexes
`computeDeltaRowCount()` also supplies `Statistics.deltaRowCount`, which `FilterEstimation.estimate()` uses to retry zero estimates with unknown column statistics. For an AGG table with rollup `r1(k1,v)`, load key 1, run FULL ANALYZE, then load 100 distinct keys 100..199 without reanalyzing. The plan `Filter(k1 >= 100) -> OlapScan(t INDEX r1)` still reads cached max=1. This check sets delta=0, so the filter stays at zero although 100 rows match; previously delta=100 triggered the retry and a nonzero estimate. This occurs even after BE reports the index row count and distorts join/distribution costing. Keep stale-statistics recovery available for these indexes independently of whether base-row deltas may be added to their row counts, and cover the analyze/load/out-of-range-filter case.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4312279362
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/analysis/TableStatsMeta.java:
##########
@@ -194,6 +273,21 @@ public void update(AnalysisInfo analyzedJob, TableIf tableIf) {
if (tableIf != null) {
if (tableIf instanceof OlapTable) {
OlapTable olapTable = (OlapTable) tableIf;
+ // The collected row counts published below already include the rows which had been loaded when
+ // the job was built, remember how many they were, they are not delta rows. The baseline may
+ // only advance together with the collected base index row count, an analysis of another index
+ // (a materialized view) doesn't touch it.
+ // NOTICE: the baseline is published before the collected counts. They are both read without
+ // the table lock by the planner, and a reader which pairs the collected count of this analysis
+ // with the baseline of the previous one counts the rows loaded since that previous analysis
+ // twice.
+ if (analyzedJob.indexesRowCount.containsKey(olapTable.getBaseIndexId())) {
+ // A row count supplied by the user replaces the collected one and describes the table as
+ // of now, so the rows loaded so far are part of it and the baseline has to move to the
+ // current value. Keeping the older baseline would count those rows again once DROP STATS
+ // clears userInjected.
+ updatedRowsBase.set(analyzedJob.userInject ? updatedRows.get() : analyzedJob.updateRows);
Review Comment:
这是老代码固有问题, 设计使用deltaRow时就已知有这个缺陷. 提了新issue 跟踪 https://github.com/apache/doris/issues/69280
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #69028:
URL: https://github.com/apache/doris/pull/69028#discussion_r4355526082
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/repository/StatisticsCleaner.java:
##########
@@ -140,6 +140,8 @@ private void clearTableStats() {
}
LOG.info("Table {}.{}.{} with id {} not exist, remove its table stats record.",
stats.ctlName, stats.dbName, stats.tblName, id);
+ // No table lock: the table is gone, so no load can recreate the record of this id while the
Review Comment:
已在 4c792cff11f 修复。
cleaner 现在对“还在回收站里”的表，会取**该回收站表对象的表写锁**（`RECOVER TABLE` 注册回的就是同一个实例，也正是 load 在 `finishTransaction` 里一直持有到记录重建完成的那把锁），把「删除记录 + 写 `DELETE_TABLE_STATS` journal」放进同一个临界区，并在锁内重新查一次 catalog：
- 删除落在 RECOVER/LOAD 之前 → load 建记录对应的 journal 排在这次删除之后，重放顺序与 master 一致；
- load 已经建好记录 → 删掉的就是这条新记录，重放时同样会删掉 → 两边一致；
- 如果在检查与加锁之间表被 RECOVER 回来 → 不再当孤儿，保留它的统计。
表彻底不在回收站（不可恢复）时仍保留原来的无锁删除，因为这种 id 之后不会有 load 再建记录。另外新增 `CatalogRecycleBin#getRecycledTable(long)` 暴露回收站里的实例。之前那句“表已经没了，不会有 load 重建记录，所以不需要表锁”的注释是错的：`RECOVER TABLE` 会用原 id 重新注册，“不在 catalog”并不等于“这个 id 不会再产生记录”。
测试：
- `StatisticsCleanerTest#testRecycledTableStatsAreRemovedUnderTheTableLock`：删除与 journal 写严格落在 `writeLock` 与 `writeUnlock` 之间；
- `StatisticsCleanerTest#testTableRecoveredBeforeTheRemovalKeepsItsStats`：锁内发现表已被恢复则不动记录、不写 journal；
- `StatisticsCleanerTest` 3/3，checkstyle 通过。
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](statistics) Create the table stats record when a load finishes [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly merged PR #69028:
URL: https://github.com/apache/doris/pull/69028
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org