This is an automated email from the ASF dual-hosted git repository.

lijibing pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/branch-2.0 by this push:
     new b7fc1885bd0 [fix](statistics)Fix drop stats log editlog bug. Catch 
drop stats exception while truncate table. (#40738) (#40979)
b7fc1885bd0 is described below

commit b7fc1885bd0a91c5b871f9b092ebb99323a88c4c
Author: Jibing-Li <64681310+jibing...@users.noreply.github.com>
AuthorDate: Thu Sep 26 11:46:12 2024 +0800

    [fix](statistics)Fix drop stats log editlog bug. Catch drop stats exception 
while truncate table. (#40738) (#40979)
    
    backport: https://github.com/apache/doris/pull/40738
---
 .../apache/doris/alter/SchemaChangeHandler.java    |  12 +-
 .../org/apache/doris/alter/SchemaChangeJobV2.java  |   6 +-
 .../main/java/org/apache/doris/catalog/Env.java    |   6 +-
 .../apache/doris/service/FrontendServiceImpl.java  |   3 -
 .../apache/doris/statistics/AnalysisManager.java   |  70 ++++----
 .../statistics/test_drop_stats_and_truncate.groovy | 179 +++++++++++++++++++++
 6 files changed, 219 insertions(+), 57 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java 
b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
index f60352f6e6b..1fd46769fbf 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
@@ -2693,11 +2693,7 @@ public class SchemaChangeHandler extends AlterHandler {
                 LOG.debug("logModifyTableAddOrDropInvertedIndices info:{}", 
info);
                 
Env.getCurrentEnv().getEditLog().logModifyTableAddOrDropInvertedIndices(info);
                 // Drop table column stats after light schema change finished.
-                try {
-                    
Env.getCurrentEnv().getAnalysisManager().dropStats(olapTable);
-                } catch (Exception e) {
-                    LOG.info("Failed to drop stats after light schema change. 
Reason: {}", e.getMessage());
-                }
+                Env.getCurrentEnv().getAnalysisManager().dropStats(olapTable);
 
                 if (isDropIndex) {
                     // send drop rpc to be
@@ -2723,11 +2719,7 @@ public class SchemaChangeHandler extends AlterHandler {
                 LOG.debug("logModifyTableAddOrDropColumns info:{}", info);
                 
Env.getCurrentEnv().getEditLog().logModifyTableAddOrDropColumns(info);
                 // Drop table column stats after light schema change finished.
-                try {
-                    
Env.getCurrentEnv().getAnalysisManager().dropStats(olapTable);
-                } catch (Exception e) {
-                    LOG.info("Failed to drop stats after light schema change. 
Reason: {}", e.getMessage());
-                }
+                Env.getCurrentEnv().getAnalysisManager().dropStats(olapTable);
             }
             LOG.info("finished modify table's add or drop or modify columns. 
table: {}, job: {}, is replay: {}",
                     olapTable.getName(), jobId, isReplay);
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java 
b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java
index f524fecdd9d..70799f0b4b6 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java
@@ -605,11 +605,7 @@ public class SchemaChangeJobV2 extends AlterJobV2 {
         changeTableState(dbId, tableId, OlapTableState.NORMAL);
         LOG.info("set table's state to NORMAL, table id: {}, job id: {}", 
tableId, jobId);
         // Drop table column stats after schema change finished.
-        try {
-            Env.getCurrentEnv().getAnalysisManager().dropStats(tbl);
-        } catch (Exception e) {
-            LOG.info("Failed to drop stats after schema change finished. 
Reason: {}", e.getMessage());
-        }
+        Env.getCurrentEnv().getAnalysisManager().dropStats(tbl);
     }
 
     private void onFinished(OlapTable tbl) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java 
b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java
index 44614c49f8b..dd65fa7cafd 100755
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java
@@ -4536,11 +4536,7 @@ public class Env {
                     indexIdToSchemaVersion);
             editLog.logColumnRename(info);
             LOG.info("rename coloumn[{}] to {}", colName, newColName);
-            try {
-                Env.getCurrentEnv().getAnalysisManager().dropStats(table);
-            } catch (Exception e) {
-                LOG.info("Failed to drop stats after rename column. Reason: 
{}", e.getMessage());
-            }
+            Env.getCurrentEnv().getAnalysisManager().dropStats(table);
         }
     }
 
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java 
b/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java
index 884065baf30..d5aef9a78e3 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java
@@ -3113,9 +3113,6 @@ public class FrontendServiceImpl implements 
FrontendService.Iface {
         InvalidateStatsTarget target = GsonUtils.GSON.fromJson(request.key, 
InvalidateStatsTarget.class);
         AnalysisManager analysisManager = 
Env.getCurrentEnv().getAnalysisManager();
         TableStatsMeta tableStats = 
analysisManager.findTableStatsStatus(target.tableId);
-        if (tableStats == null) {
-            return new TStatus(TStatusCode.OK);
-        }
         analysisManager.invalidateLocalStats(target.catalogId, target.dbId, 
target.tableId, target.columns, tableStats);
         return new TStatus(TStatusCode.OK);
     }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/statistics/AnalysisManager.java 
b/fe/fe-core/src/main/java/org/apache/doris/statistics/AnalysisManager.java
index 814fc6eb05b..af17a63da20 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/statistics/AnalysisManager.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/statistics/AnalysisManager.java
@@ -691,13 +691,18 @@ public class AnalysisManager implements Writable {
             return;
         }
 
+        TableStatsMeta tableStats = 
findTableStatsStatus(dropStatsStmt.getTblId());
+        if (tableStats == null) {
+            return;
+        }
         Set<String> cols = dropStatsStmt.getColumnNames();
         long catalogId = dropStatsStmt.getCatalogIdId();
         long dbId = dropStatsStmt.getDbId();
         long tblId = dropStatsStmt.getTblId();
-        TableStatsMeta tableStats = 
findTableStatsStatus(dropStatsStmt.getTblId());
-        if (tableStats == null) {
-            return;
+        // Remove tableMetaStats if drop whole table stats.
+        if (dropStatsStmt.isAllColumns()) {
+            removeTableStats(tblId);
+            Env.getCurrentEnv().getEditLog().logDeleteTableStats(new 
TableStatsDeletionLog(tblId));
         }
         invalidateLocalStats(catalogId, dbId, tblId, 
dropStatsStmt.isAllColumns() ? null : cols, tableStats);
         // Drop stats ddl is master only operation.
@@ -705,19 +710,26 @@ public class AnalysisManager implements Writable {
         StatisticsRepository.dropStatistics(tblId, cols);
     }
 
-    public void dropStats(TableIf table) throws DdlException {
-        TableStatsMeta tableStats = findTableStatsStatus(table.getId());
-        if (tableStats == null) {
-            return;
+    public void dropStats(TableIf table) {
+        try {
+            TableStatsMeta tableStats = findTableStatsStatus(table.getId());
+            if (tableStats == null) {
+                return;
+            }
+            long catalogId = table.getDatabase().getCatalog().getId();
+            long dbId = table.getDatabase().getId();
+            long tableId = table.getId();
+            removeTableStats(tableId);
+            Env.getCurrentEnv().getEditLog().logDeleteTableStats(new 
TableStatsDeletionLog(tableId));
+            Set<String> cols = 
table.getSchemaAllIndexes(false).stream().map(Column::getName)
+                    .collect(Collectors.toSet());
+            invalidateLocalStats(catalogId, dbId, tableId, null, tableStats);
+            // Drop stats ddl is master only operation.
+            invalidateRemoteStats(catalogId, dbId, tableId, cols, true);
+            StatisticsRepository.dropStatistics(table.getId(), cols);
+        } catch (Throwable e) {
+            LOG.warn("Failed to drop stats for table {}", table.getName(), e);
         }
-        long catalogId = table.getDatabase().getCatalog().getId();
-        long dbId = table.getDatabase().getId();
-        long tableId = table.getId();
-        Set<String> cols = 
table.getSchemaAllIndexes(false).stream().map(Column::getName).collect(Collectors.toSet());
-        invalidateLocalStats(catalogId, dbId, tableId, null, tableStats);
-        // Drop stats ddl is master only operation.
-        invalidateRemoteStats(catalogId, dbId, tableId, cols, true);
-        StatisticsRepository.dropStatistics(table.getId(), cols);
     }
 
     public void dropCachedStats(long catalogId, long dbId, long tableId) {
@@ -740,14 +752,9 @@ public class AnalysisManager implements Writable {
 
     public void invalidateLocalStats(long catalogId, long dbId, long tableId,
             Set<String> columns, TableStatsMeta tableStats) {
-        if (tableStats == null) {
-            return;
-        }
         TableIf table = StatisticsUtil.findTable(catalogId, dbId, tableId);
         StatisticsCache statsCache = Env.getCurrentEnv().getStatisticsCache();
-        boolean allColumn = false;
         if (columns == null) {
-            allColumn = true;
             columns = table.getSchemaAllIndexes(false)
                 .stream().map(Column::getName).collect(Collectors.toSet());
         }
@@ -760,18 +767,16 @@ public class AnalysisManager implements Writable {
                 indexIds.add(-1L);
             }
             for (long indexId : indexIds) {
-                tableStats.removeColumn(column);
+                if (tableStats != null) {
+                    tableStats.removeColumn(column);
+                }
                 statsCache.invalidate(tableId, indexId, column);
             }
         }
-        // To remove stale column name that is changed before.
-        if (allColumn) {
-            tableStats.removeAllColumn();
-            tableStats.clearIndexesRowCount();
-            removeTableStats(tableId);
+        if (tableStats != null) {
+            tableStats.updatedTime = 0;
+            tableStats.userInjected = false;
         }
-        tableStats.updatedTime = 0;
-        tableStats.userInjected = false;
     }
 
     public void invalidateRemoteStats(long catalogId, long dbId, long tableId,
@@ -781,18 +786,15 @@ public class AnalysisManager implements Writable {
         request.key = GsonUtils.GSON.toJson(target);
         StatisticsCache statisticsCache = 
Env.getCurrentEnv().getStatisticsCache();
         SystemInfoService.HostInfo selfNode = 
Env.getCurrentEnv().getSelfNode();
-        boolean success = true;
         for (Frontend frontend : Env.getCurrentEnv().getFrontends(null)) {
             // Skip master
             if (selfNode.getHost().equals(frontend.getHost())) {
                 continue;
             }
-            success = success && statisticsCache.invalidateStats(frontend, 
request);
+            statisticsCache.invalidateStats(frontend, request);
         }
-        if (!success) {
-            // If any rpc failed, use edit log to sync table stats to 
non-master FEs.
-            LOG.warn("Failed to invalidate all remote stats by rpc for table 
{}, use edit log.", tableId);
-            TableStatsMeta tableStats = findTableStatsStatus(tableId);
+        TableStatsMeta tableStats = findTableStatsStatus(tableId);
+        if (tableStats != null) {
             logCreateTableStats(tableStats);
         }
     }
diff --git 
a/regression-test/suites/statistics/test_drop_stats_and_truncate.groovy 
b/regression-test/suites/statistics/test_drop_stats_and_truncate.groovy
new file mode 100644
index 00000000000..85d04aba993
--- /dev/null
+++ b/regression-test/suites/statistics/test_drop_stats_and_truncate.groovy
@@ -0,0 +1,179 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_drop_stats_and_truncate") {
+
+    sql """drop database if exists test_drop_stats_and_truncate"""
+    sql """create database test_drop_stats_and_truncate"""
+    sql """use test_drop_stats_and_truncate"""
+    sql """set global enable_auto_analyze=false"""
+
+    sql """CREATE TABLE non_part  (
+            r_regionkey      int NOT NULL,
+            r_name       VARCHAR(25) NOT NULL,
+            r_comment    VARCHAR(152)
+        )ENGINE=OLAP
+        DUPLICATE KEY(`r_regionkey`)
+        COMMENT "OLAP"
+        DISTRIBUTED BY HASH(`r_regionkey`) BUCKETS 1
+        PROPERTIES (
+            "replication_num" = "1"
+        );
+    """
+    sql """CREATE TABLE `part` (
+            `id` INT NULL,
+            `colint` INT NULL,
+            `coltinyint` int NULL,
+            `colsmallint` smallINT NULL,
+            `colbigint` bigINT NULL,
+            `collargeint` largeINT NULL,
+            `colfloat` float NULL,
+            `coldouble` double NULL,
+            `coldecimal` decimal(27, 9) NULL
+        ) ENGINE=OLAP
+        DUPLICATE KEY(`id`)
+        COMMENT 'OLAP'
+        PARTITION BY RANGE(`id`)
+        (
+            PARTITION p1 VALUES [("-2147483648"), ("10000")),
+            PARTITION p2 VALUES [("10000"), ("20000")),
+            PARTITION p3 VALUES [("20000"), ("30000"))
+        )
+        DISTRIBUTED BY HASH(`id`) BUCKETS 3
+        PROPERTIES (
+            "replication_allocation" = "tag.location.default: 1"
+        )
+    """
+    sql """insert into non_part values (1, "1", "1");"""
+    sql """analyze table non_part with sync"""
+
+    def result = sql """show column cached stats non_part"""
+    assertEquals(3, result.size())
+    result = sql """show column stats non_part"""
+    assertEquals(3, result.size())
+    result = sql """show table stats non_part"""
+    def all_columns = result[0][4]
+    String[] columns = all_columns.split(",");
+    assertEquals(3, columns.size())
+
+    sql """drop stats non_part(r_comment)"""
+    result = sql """show column cached stats non_part"""
+    assertEquals(2, result.size())
+    result = sql """show column stats non_part"""
+    assertEquals(2, result.size())
+    result = sql """show table stats non_part"""
+    all_columns = result[0][4]
+    columns = all_columns.split(",");
+    assertEquals(2, columns.size())
+
+    sql """drop stats non_part"""
+    result = sql """show column cached stats non_part"""
+    assertEquals(0, result.size())
+    result = sql """show column stats non_part"""
+    assertEquals(0, result.size())
+    result = sql """show table stats non_part"""
+    all_columns = result[0][4]
+    assertEquals("", all_columns)
+
+    sql """analyze table non_part with sync"""
+    result = sql """show column cached stats non_part"""
+    assertEquals(3, result.size())
+    result = sql """show column stats non_part"""
+    assertEquals(3, result.size())
+    result = sql """show table stats non_part"""
+    all_columns = result[0][4]
+    columns = all_columns.split(",");
+    assertEquals(3, columns.size())
+
+    sql """truncate table non_part"""
+    result = sql """show column stats non_part"""
+    assertEquals(0, result.size())
+    result = sql """show table stats non_part"""
+    all_columns = result[0][4]
+    assertEquals("", all_columns)
+
+    sql """Insert into part values (1, 1, 1, 1, 1, 1, 1.1, 1.1, 1.1), (2, 2, 
2, 2, 2, 2, 2.2, 2.2, 2.2), (3, 3, 3, 3, 3, 3, 3.3, 3.3, 3.3),(4, 4, 4, 4, 4, 
4, 4.4, 4.4, 4.4),(5, 5, 5, 5, 5, 5, 5.5, 5.5, 5.5),(6, 6, 6, 6, 6, 6, 6.6, 
6.6, 6.6),(10001, 10001, 10001, 10001, 10001, 10001, 10001.10001, 10001.10001, 
10001.10001),(10002, 10002, 10002, 10002, 10002, 10002, 10002.10002, 
10002.10002, 10002.10002),(10003, 10003, 10003, 10003, 10003, 10003, 
10003.10003, 10003.10003, 10003.10003),(1000 [...]
+    sql """analyze table part with sync"""
+    result = sql """show column cached stats part"""
+    assertEquals(9, result.size())
+    result = sql """show column stats part"""
+    assertEquals(9, result.size())
+    result = sql """show table stats part"""
+    all_columns = result[0][4]
+    columns = all_columns.split(",");
+    assertEquals(9, columns.size())
+
+    sql """drop stats part(colint)"""
+    result = sql """show column cached stats part"""
+    assertEquals(8, result.size())
+    result = sql """show column stats part"""
+    assertEquals(8, result.size())
+    result = sql """show table stats part"""
+    all_columns = result[0][4]
+    columns = all_columns.split(",");
+    assertEquals(8, columns.size())
+
+    sql """drop stats part"""
+    result = sql """show column cached stats part"""
+    assertEquals(0, result.size())
+    result = sql """show column stats part"""
+    assertEquals(0, result.size())
+    result = sql """show table stats part"""
+    all_columns = result[0][4]
+    assertEquals("", all_columns)
+
+    sql """analyze table part with sync"""
+    result = sql """show column cached stats part"""
+    assertEquals(9, result.size())
+    result = sql """show column stats part"""
+    assertEquals(9, result.size())
+    result = sql """show table stats part"""
+    all_columns = result[0][4]
+    columns = all_columns.split(",");
+    assertEquals(9, columns.size())
+
+    sql """truncate table part"""
+    result = sql """show column stats part"""
+    assertEquals(0, result.size())
+    result = sql """show table stats part"""
+    all_columns = result[0][4]
+    assertEquals("", all_columns)
+
+    sql """Insert into part values (1, 1, 1, 1, 1, 1, 1.1, 1.1, 1.1), (2, 2, 
2, 2, 2, 2, 2.2, 2.2, 2.2), (3, 3, 3, 3, 3, 3, 3.3, 3.3, 3.3),(4, 4, 4, 4, 4, 
4, 4.4, 4.4, 4.4),(5, 5, 5, 5, 5, 5, 5.5, 5.5, 5.5),(6, 6, 6, 6, 6, 6, 6.6, 
6.6, 6.6),(10001, 10001, 10001, 10001, 10001, 10001, 10001.10001, 10001.10001, 
10001.10001),(10002, 10002, 10002, 10002, 10002, 10002, 10002.10002, 
10002.10002, 10002.10002),(10003, 10003, 10003, 10003, 10003, 10003, 
10003.10003, 10003.10003, 10003.10003),(1000 [...]
+    sql """analyze table part with sync"""
+    result = sql """show column cached stats part"""
+    assertEquals(9, result.size())
+    result = sql """show column stats part"""
+    assertEquals(9, result.size())
+    result = sql """show table stats part"""
+    all_columns = result[0][4]
+    columns = all_columns.split(",");
+    assertEquals(9, columns.size())
+
+    sql """truncate table part partition(p1)"""
+    result = sql """show column cached stats part"""
+    assertEquals(9, result.size())
+    result = sql """show column stats part"""
+    assertEquals(9, result.size())
+    result = sql """show table stats part"""
+    all_columns = result[0][4]
+    columns = all_columns.split(",");
+    assertEquals(9, columns.size())
+
+    sql """drop database if exists test_drop_stats_and_truncate"""
+}
+


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to