From 1fb771d526ea56a3c24effaf8728492d7e2e357e Mon Sep 17 00:00:00 2001 From: James Date: Sun, 23 Mar 2025 09:55:47 +0800 Subject: [PATCH] branch-2.1: [fix](statistics)Fix replace table doesn't remove table stat meta memory leak bug. (#49345) (#49367) Cherry-picked from https://github.com/apache/doris/pull/49345 --- .../java/org/apache/doris/alter/Alter.java | 1 + .../doris/statistics/StatisticsCleaner.java | 19 ++++++++++++------- ...table.grovvy => test_replace_table.groovy} | 18 ++++++++++++------ 3 files changed, 25 insertions(+), 13 deletions(-) rename regression-test/suites/statistics/{test_replace_table.grovvy => test_replace_table.groovy} (82%) diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java index 5e6ae15895..61d51dcc8a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java @@ -685,6 +685,7 @@ public class Alter { if (origTable.getType() == TableType.MATERIALIZED_VIEW) { Env.getCurrentEnv().getMtmvService().deregisterMTMV((MTMV) origTable); } + Env.getCurrentEnv().getAnalysisManager().removeTableStats(origTable.getId()); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsCleaner.java b/fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsCleaner.java index a61d08f7d8..578420cf97 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsCleaner.java +++ b/fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsCleaner.java @@ -116,19 +116,24 @@ public class StatisticsCleaner extends MasterDaemon { // If ctlName, dbName and tblName exist, it means the table stats is created under new version. // First try to find the table by the given names. If table exists, means the tableMeta is valid, // it should be kept in memory. + boolean tableExist = false; try { - StatisticsUtil.findTable(stats.ctlName, stats.dbName, stats.tblName); - continue; + TableIf table = StatisticsUtil.findTable(stats.ctlName, stats.dbName, stats.tblName); + // Tables may have identical names but different id, e.g. replace table. + tableExist = table.getId() == id; } catch (Exception e) { LOG.debug("Table {}.{}.{} not found.", stats.ctlName, stats.dbName, stats.tblName); } - // If we couldn't find table by names, try to find it in internal catalog. This is to support older - // version which the tableStats object doesn't store the names but only table id. + // If we couldn't find table by names, try to find it in internal catalog by id. + // This is to support older version which the tableStats object doesn't store the names + // but only table id. // We may remove external table's tableStats here, but it's not a big problem. // Because the stats in column_statistics table is still available, - // the only disadvantage is auto analyze may be triggered for this table. - // But it only happens once, the new table stats object will have all the catalog, db and table names. - if (tableExistInInternalCatalog(internalCatalog, id)) { + // the only disadvantage is auto analyze may be triggered for this table again. + // But it only happens once, the new table stats object will have all the catalog, + // db and table names. + // Also support REPLACE TABLE + if (tableExist || tableExistInInternalCatalog(internalCatalog, id)) { continue; } LOG.info("Table {}.{}.{} with id {} not exist, remove its table stats record.", diff --git a/regression-test/suites/statistics/test_replace_table.grovvy b/regression-test/suites/statistics/test_replace_table.groovy similarity index 82% rename from regression-test/suites/statistics/test_replace_table.grovvy rename to regression-test/suites/statistics/test_replace_table.groovy index 08c68c5ac2..cb8053cc7c 100644 --- a/regression-test/suites/statistics/test_replace_table.grovvy +++ b/regression-test/suites/statistics/test_replace_table.groovy @@ -17,10 +17,9 @@ suite("test_replace_table") { - sql """drop database if exists test_replace_table""" - sql """create database test_replace_table""" - sql """use test_replace_table""" - sql """set global force_sample_analyze=false""" + sql """drop database if exists test_replace_table_statistics""" + sql """create database test_replace_table_statistics""" + sql """use test_replace_table_statistics""" sql """set global enable_auto_analyze=false""" sql """CREATE TABLE t1 ( @@ -70,12 +69,19 @@ suite("test_replace_table") { result = sql """show column cached stats t2""" assertEquals(2, result.size()) + def id1 = get_table_id("internal", "test_replace_table_statistics", "t1") + def id2 = get_table_id("internal", "test_replace_table_statistics", "t2") + sql """ALTER TABLE t1 REPLACE WITH TABLE t2 PROPERTIES('swap' = 'false');""" result = sql """show column stats t1""" assertEquals(2, result.size()) result = sql """show column cached stats t1""" assertEquals(2, result.size()) - sql """drop database if exists test_replace_table""" -} + result = sql """show table stats ${id1}""" + assertEquals(0, result.size()) + result = sql """show table stats ${id2}""" + assertEquals(1, result.size()) + sql """drop database if exists test_replace_table_statistics""" +}