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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to