From aac2b7c6e2763ee7b23475b90f72e2070d67445e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E4=B8=87=E4=B9=89?= Date: Sat, 29 Aug 2026 22:51:02 +0800 Subject: [PATCH] [BugFix] Fix ABA race in eraseTable and erasePartition (#67303) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 张万义 --- .../doris/catalog/CatalogRecycleBin.java | 54 ++++++++++++++----- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/CatalogRecycleBin.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/CatalogRecycleBin.java index ca6f332a99531f..4fee58a48fb2b2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/CatalogRecycleBin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/CatalogRecycleBin.java @@ -295,13 +295,13 @@ private void eraseDatabase(long currentTimeMs, int keepNum) { int eraseNum = 0; StopWatch watch = StopWatch.createStarted(); try { - // 1. collect expired database IDs under read lock - List expiredIds = new ArrayList<>(); + // 1. collect expired database IDs and recycle times under read lock + Map expiredIdToRecycleTime = new HashMap<>(); readLock(); try { for (Map.Entry entry : idToDatabase.entrySet()) { if (isExpire(entry.getKey(), currentTimeMs)) { - expiredIds.add(entry.getKey()); + expiredIdToRecycleTime.put(entry.getKey(), idToRecycleTime.get(entry.getKey())); } } } finally { @@ -309,13 +309,22 @@ private void eraseDatabase(long currentTimeMs, int keepNum) { } // 2. erase each expired database one at a time - for (Long dbId : expiredIds) { + for (Map.Entry expiredEntry : expiredIdToRecycleTime.entrySet()) { + Long dbId = expiredEntry.getKey(); + Long capturedRecycleTime = expiredEntry.getValue(); writeLock(); try { - RecycleDatabaseInfo dbInfo = idToDatabase.remove(dbId); + RecycleDatabaseInfo dbInfo = idToDatabase.get(dbId); if (dbInfo == null) { continue; } + // Re-validate that the recycle time hasn't changed and the entry is still expired + Long currentRecycleTime = idToRecycleTime.get(dbId); + if (currentRecycleTime == null || !currentRecycleTime.equals(capturedRecycleTime) + || !isExpire(dbId, currentTimeMs)) { + continue; + } + idToDatabase.remove(dbId); Database db = dbInfo.getDb(); idToRecycleTime.remove(dbId); @@ -457,13 +466,13 @@ private void eraseTable(long currentTimeMs, int keepNum) { int eraseNum = 0; StopWatch watch = StopWatch.createStarted(); try { - // 1. collect expired table IDs under read lock - List expiredIds = new ArrayList<>(); + // 1. collect expired table IDs and recycle times under read lock + Map expiredIdToRecycleTime = new HashMap<>(); readLock(); try { for (Map.Entry entry : idToTable.entrySet()) { if (isExpire(entry.getKey(), currentTimeMs)) { - expiredIds.add(entry.getKey()); + expiredIdToRecycleTime.put(entry.getKey(), idToRecycleTime.get(entry.getKey())); } } } finally { @@ -471,13 +480,21 @@ private void eraseTable(long currentTimeMs, int keepNum) { } // 2. erase each expired table one at a time - for (Long tableId : expiredIds) { + for (Map.Entry expiredEntry : expiredIdToRecycleTime.entrySet()) { + Long tableId = expiredEntry.getKey(); + Long capturedRecycleTime = expiredEntry.getValue(); writeLock(); try { RecycleTableInfo tableInfo = idToTable.get(tableId); if (tableInfo == null) { continue; } + // Re-validate that the recycle time hasn't changed and the entry is still expired + Long currentRecycleTime = idToRecycleTime.get(tableId); + if (currentRecycleTime == null || !currentRecycleTime.equals(capturedRecycleTime) + || !isExpire(tableId, currentTimeMs)) { + continue; + } Table table = tableInfo.getTable(); try { Env.getCurrentInternalCatalog().beforeEraseTable(tableInfo.dbId, table, false); @@ -602,13 +619,13 @@ private void erasePartition(long currentTimeMs, int keepNum) { int eraseNum = 0; StopWatch watch = StopWatch.createStarted(); try { - // 1. collect expired partition IDs under read lock - List expiredIds = new ArrayList<>(); + // 1. collect expired partition IDs and recycle times under read lock + Map expiredIdToRecycleTime = new HashMap<>(); readLock(); try { for (Map.Entry entry : idToPartition.entrySet()) { if (isExpire(entry.getKey(), currentTimeMs)) { - expiredIds.add(entry.getKey()); + expiredIdToRecycleTime.put(entry.getKey(), idToRecycleTime.get(entry.getKey())); } } } finally { @@ -616,15 +633,24 @@ private void erasePartition(long currentTimeMs, int keepNum) { } // 2. erase each expired partition one at a time (microbatch) - for (Long partitionId : expiredIds) { + for (Map.Entry expiredEntry : expiredIdToRecycleTime.entrySet()) { + Long partitionId = expiredEntry.getKey(); + Long capturedRecycleTime = expiredEntry.getValue(); writeLock(); try { - RecyclePartitionInfo partitionInfo = idToPartition.remove(partitionId); + RecyclePartitionInfo partitionInfo = idToPartition.get(partitionId); if (partitionInfo == null) { continue; } + // Re-validate that the recycle time hasn't changed and the entry is still expired + Long currentRecycleTime = idToRecycleTime.get(partitionId); + if (currentRecycleTime == null || !currentRecycleTime.equals(capturedRecycleTime) + || !isExpire(partitionId, currentTimeMs)) { + continue; + } Partition partition = partitionInfo.getPartition(); Env.getCurrentEnv().onErasePartition(partition); + idToPartition.remove(partitionId); idToRecycleTime.remove(partitionId); dbTblIdPartitionNameToIds.computeIfPresent(