[Bug] CatalogRecycleBin microbatch erase has an ABA race and retains a long-held lock in DB cascade cleanup

[wenzhenghu]

Search before asking

• I searched the existing issues and pull requests. [Enhancement] Recycle Bin Thread Lock Contention Causes High Latency in Drop Partition Operations #48224 reported the earlier general recycle-bin lock contention, but it was closed as stale and does not cover the ABA correctness race or the remaining database-cascade lock scope after [Enhancement](recyclebin) Optimize lock granularity in CatalogRecycleBin #61366.

Version

Apache Doris master at 2689e0d7fdb111bf822cebc26d7d5765a563e276 (2026-08-28).

The behavior was introduced/retained by #61366 (e2a678cb74a5ef6278a7c968901c2de9348e0626) and is still present on the current master.

What's Wrong?

#61366 changed CatalogRecycleBin from a coarse monitor to a ReentrantReadWriteLock and introduced per-item erase processing. Two problems remain.

1. Correctness: stale expired-ID snapshots can erase a newly recycled generation (ABA race)

The normal expired database/table/partition paths:

1. Collect only expired object IDs under the read lock.
2. Release the read lock.
3. Later acquire the write lock and erase whichever Recycle*Info is currently mapped to that ID.

There is no validation that the current Recycle*Info instance and recycle timestamp are the same generation observed during the expired scan, and expiration is not checked again after acquiring the write lock.

Current code:

• Database:

  doris/fe/fe-core/src/main/java/org/apache/doris/catalog/CatalogRecycleBin.java

  Lines 298 to 333 in 2689e0d

  	// 1. collect expired database IDs under read lock
  	List<Long> expiredIds = new ArrayList<>();
  	readLock();
  	try {
  	for (Map.Entry<Long, RecycleDatabaseInfo> entry : idToDatabase.entrySet()) {
  	if (isExpire(entry.getKey(), currentTimeMs)) {
  	expiredIds.add(entry.getKey());
  	}
  	}
  	} finally {
  	readUnlock();
  	}
  	// 2. erase each expired database one at a time
  	for (Long dbId : expiredIds) {
  	writeLock();
  	try {
  	RecycleDatabaseInfo dbInfo = idToDatabase.remove(dbId);
  	if (dbInfo == null) {
  	continue;
  	}
  	Database db = dbInfo.getDb();
  	idToRecycleTime.remove(dbId);
  	dbNameToIds.computeIfPresent(db.getFullName(), (k, v) -> {
  	v.remove(db.getId());
  	return v.isEmpty() ? null : v;
  	});
  	Env.getCurrentEnv().eraseDatabase(db.getId(), true);
  	LOG.info("erase db[{}]", db.getId());
  	eraseNum++;
  	} finally {
  	writeUnlock();
  	}
  	}

• Table:

  doris/fe/fe-core/src/main/java/org/apache/doris/catalog/CatalogRecycleBin.java

  Lines 460 to 507 in 2689e0d

  	// 1. collect expired table IDs under read lock
  	List<Long> expiredIds = new ArrayList<>();
  	readLock();
  	try {
  	for (Map.Entry<Long, RecycleTableInfo> entry : idToTable.entrySet()) {
  	if (isExpire(entry.getKey(), currentTimeMs)) {
  	expiredIds.add(entry.getKey());
  	}
  	}
  	} finally {
  	readUnlock();
  	}
  	// 2. erase each expired table one at a time
  	for (Long tableId : expiredIds) {
  	writeLock();
  	try {
  	RecycleTableInfo tableInfo = idToTable.get(tableId);
  	if (tableInfo == null) {
  	continue;
  	}
  	Table table = tableInfo.getTable();
  	try {
  	Env.getCurrentInternalCatalog().beforeEraseTable(tableInfo.dbId, table, false);
  	} catch (DdlException e) {
  	LOG.warn("failed to create erase task for table {}", tableId, e);
  	continue;
  	}
  	if (table.isManagedTable()) {
  	Env.getCurrentEnv().onEraseOlapTable(tableInfo.dbId, (OlapTable) table, false);
  	}
  	idToTable.remove(tableId);
  	idToRecycleTime.remove(tableId);
  	dbIdTableNameToIds.computeIfPresent(Pair.of(tableInfo.getDbId(), table.getName()),
  	(k, v) -> {
  	v.remove(tableId);
  	return v.isEmpty() ? null : v;
  	});
  	Env.getCurrentEnv().getEditLog().logEraseTable(tableId);
  	LOG.info("erase table[{}]", tableId);
  	eraseNum++;
  	} finally {
  	writeUnlock();
  	}
  	}

• Partition:

  doris/fe/fe-core/src/main/java/org/apache/doris/catalog/CatalogRecycleBin.java

  Lines 605 to 645 in 2689e0d

  	// 1. collect expired partition IDs under read lock
  	List<Long> expiredIds = new ArrayList<>();
  	readLock();
  	try {
  	for (Map.Entry<Long, RecyclePartitionInfo> entry : idToPartition.entrySet()) {
  	if (isExpire(entry.getKey(), currentTimeMs)) {
  	expiredIds.add(entry.getKey());
  	}
  	}
  	} finally {
  	readUnlock();
  	}
  	// 2. erase each expired partition one at a time (microbatch)
  	for (Long partitionId : expiredIds) {
  	writeLock();
  	try {
  	RecyclePartitionInfo partitionInfo = idToPartition.remove(partitionId);
  	if (partitionInfo == null) {
  	continue;
  	}
  	Partition partition = partitionInfo.getPartition();
  	Env.getCurrentEnv().onErasePartition(partition);
  	idToRecycleTime.remove(partitionId);
  	dbTblIdPartitionNameToIds.computeIfPresent(
  	Pair.of(partitionInfo.getDbId(), partitionInfo.getTableId()), (pair, partitionMap) -> {
  	partitionMap.computeIfPresent(partition.getName(), (name, idSet) -> {
  	idSet.remove(partitionId);
  	return idSet.isEmpty() ? null : idSet;
  	});
  	return partitionMap.isEmpty() ? null : partitionMap;
  	});
  	Env.getCurrentEnv().getEditLog().logErasePartition(partitionId);
  	LOG.info("erase partition[{}]. reason: expired", partitionId);
  	eraseNum++;
  	} finally {
  	writeUnlock();
  	}
  	}

A possible interleaving is:

1. The recycle-bin daemon scans an expired entry with ID X and stores X in expiredIds.
2. A concurrent DDL recovers X, removing the old recycle-bin entry.
3. The object is dropped again. Recovery preserves the metadata ID, so this creates a new Recycle*Info for ID X with a fresh recycle timestamp.
4. The daemon processes its stale ID snapshot and erases the new entry without checking its identity or fresh timestamp.

This can bypass the configured retention window and make a newly dropped database/table/partition unrecoverable. The erase operation is also journaled, so the wrong deletion is not merely an in-memory transient.

The table path was later changed to call get(tableId) before cleanup and remove(tableId) afterward, but it still operates on the current generation without revalidating the snapshot or expiration, so the race remains.

2. Lock granularity: same-name database cleanup still erases all child tables under one write lock

eraseDatabaseWithSameName() acquires the recycle-bin write lock and calls eraseAllTables(). While that lock is held, eraseAllTables() scans idToTable and, for every matching table, performs beforeEraseTable, onEraseOlapTable, map removals, and logEraseTable:

doris/fe/fe-core/src/main/java/org/apache/doris/catalog/CatalogRecycleBin.java

Lines 356 to 434 in 2689e0d

	private void eraseDatabaseWithSameName(String dbName, long currentTimeMs,
	int maxSameNameTrashNum, List<Long> sameNameDbIdList) {
	List<Long> dbIdToErase;
	readLock();
	try {
	dbIdToErase = getIdListToEraseByRecycleTime(sameNameDbIdList, maxSameNameTrashNum);
	} finally {
	readUnlock();
	}
	for (Long dbId : dbIdToErase) {
	writeLock();
	try {
	RecycleDatabaseInfo dbInfo = idToDatabase.get(dbId);
	if (dbInfo == null || !isExpireMinLatency(dbId, currentTimeMs)) {
	continue;
	}
	if (!eraseAllTables(dbInfo)) {
	continue;
	}
	idToDatabase.remove(dbId);
	idToRecycleTime.remove(dbId);
	dbNameToIds.computeIfPresent(dbName, (k, v) -> {
	v.remove(dbId);
	return v.isEmpty() ? null : v;
	});
	Env.getCurrentEnv().eraseDatabase(dbId, true);
	LOG.info("erase database[{}] name: {}", dbId, dbName);
	} finally {
	writeUnlock();
	}
	}
	}
	private boolean isExpireMinLatency(long id, long currentTimeMs) {
	return (currentTimeMs - idToRecycleTime.get(id)) > minEraseLatency || FeConstants.runningUnitTest;
	}
	private boolean eraseAllTables(RecycleDatabaseInfo dbInfo) {
	Database db = dbInfo.getDb();
	Set<String> tableNames = Sets.newHashSet(dbInfo.getTableNames());
	Set<Long> tableIds = Sets.newHashSet(dbInfo.getTableIds());
	long dbId = db.getId();
	boolean allEraseTasksCreated = true;
	Iterator<Map.Entry<Long, RecycleTableInfo>> iterator = idToTable.entrySet().iterator();
	while (iterator.hasNext() && !tableNames.isEmpty()) {
	Map.Entry<Long, RecycleTableInfo> entry = iterator.next();
	RecycleTableInfo tableInfo = entry.getValue();
	if (tableInfo.getDbId() != dbId || !tableNames.contains(tableInfo.getTable().getName())
	|| !tableIds.contains(tableInfo.getTable().getId())) {
	continue;
	}
	Table table = tableInfo.getTable();
	try {
	Env.getCurrentInternalCatalog().beforeEraseTable(dbId, table, false);
	} catch (DdlException e) {
	LOG.warn("failed to create erase task for table {} in db {}", table.getId(), dbId, e);
	allEraseTasksCreated = false;
	continue;
	}
	if (table.isManagedTable()) {
	Env.getCurrentEnv().onEraseOlapTable(dbId, (OlapTable) table, false);
	}
	iterator.remove();
	idToRecycleTime.remove(table.getId());
	tableNames.remove(table.getName());
	dbIdTableNameToIds.computeIfPresent(Pair.of(tableInfo.getDbId(), table.getName()), (k, v) -> {
	v.remove(table.getId());
	return v.isEmpty() ? null : v;
	});
	Env.getCurrentEnv().getEditLog().logEraseTable(table.getId());
	LOG.info("erase db[{}] with table[{}]: {}", dbId, table.getId(), table.getName());
	}
	return allEraseTasksCreated;
	}

Consequently, an old same-name database containing many tables can still hold the global recycle-bin write lock for O(idToTable size + child-table count * per-table cleanup cost). Concurrent DROP/recycle operations can wait for the entire database cascade, including when their callers already hold database/table metadata locks. This retains the lock-amplification risk that the microbatch change intended to remove.

The later #65859 added beforeEraseTable() to this loop but did not split the lock scope. The currently open #61504 also changes retention behavior without addressing either concurrency pattern.

What You Expected?

1. An expired-scan result must identify the exact generation that was observed. After reacquiring the write lock, the daemon should verify that the current Recycle*Info and recycle timestamp still match the snapshot and that the entry is still expired before performing any cleanup. If it was recovered/recycled or its timestamp changed, the stale work item should be skipped.
2. Same-name database cascade cleanup should release the recycle-bin write lock between child tables, while using an explicit deletion reservation/state (or an equivalent protocol) to prevent recoverDatabase from observing or recovering a partially erased database. The database erase journal should be written only after its child cleanup has completed consistently.

The same generation-validation rule should be applied to database, table, and partition paths.

How to Reproduce?

The ABA race can be covered deterministically with an FE unit-test hook/latch:

1. Put an expired database, table, or partition in CatalogRecycleBin.
2. Start runAfterCatalogReady() and pause it after the expired snapshot is collected but before the per-item write lock is acquired.
3. Recover the object and recycle/drop the same object again, preserving its ID and recording a fresh recycle timestamp.
4. Resume the daemon.
5. Verify that the new recycle-bin entry and its fresh timestamp remain and that no erase journal was produced for the new generation.

This test should be repeated for database, table, and partition paths.

The database-cascade lock issue can also be demonstrated deterministically:

1. Create enough same-name recycled databases to exceed max_same_name_num, with one old database containing many recycled tables.
2. Block or slow the first per-table cleanup callback while eraseDatabaseWithSameName() is running.
3. Concurrently invoke another recycle-bin write operation such as recyclePartition().
4. Observe that it cannot acquire the recycle-bin write lock until all child tables of that database have been processed.

The existing testMicrobatchEraseReleasesLockBetweenItems relies on Thread.sleep(50) and does not establish that the daemon is still in the erase iteration when the concurrent recycle runs:

https://github.com/apache/doris/blob/2689e0d7fdb111bf822cebc26d7d5765a563e276/fe/fe-core/src/test/java/org/apache/doris/catalog/CatalogRecycleBinTest.java#L989-L1050

Anything Else?

Related history:

• Earlier general lock-contention report: [Enhancement] Recycle Bin Thread Lock Contention Causes High Latency in Drop Partition Operations #48224
• Lock-granularity/microbatch implementation: [Enhancement](recyclebin) Optimize lock granularity in CatalogRecycleBin #61366
• Later code added more per-table work inside the same database-cascade write-lock scope: [feature](tablet stream) Support Cloud Table Stream control plane #65859
• Open retention redesign that currently retains these patterns: [feature](recycle-bin) support two-phase retention for recycle bin #61504

This report is based on source and history analysis of the current master. A deterministic race test has not yet been run.

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[zhang-arvin]

I will work on this.