Skip to content

branch-4.1: [fix](fe) Prevent stale external name publication after refresh #67417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
924060929 wants to merge 1 commit into
base: branch-4.1
Choose a base branch
from
+542 −55
Open

branch-4.1: [fix](fe) Prevent stale external name publication after refresh #67417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/common/CacheFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.github.benmanes.caffeine.cache.AsyncCacheLoader;
import com.github.benmanes.caffeine.cache.AsyncLoadingCache;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.CacheLoader;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.LoadingCache;
Expand Down Expand Up @@ -95,6 +96,11 @@ public CacheFactory withSoftValues() {
return this;
}

// Build a cache without automatic loading.
public <K, V> Cache<K, V> buildCache() {
return buildWithParams().build();
}

// Build a loading cache, without executor, it will use fork-join pool for refresh
public <K, V> LoadingCache<K, V> buildCache(CacheLoader<K, V> cacheLoader) {
Caffeine<Object, Object> builder = buildWithParams();
Expand Down
22 changes: 13 additions & 9 deletions fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public abstract class ExternalCatalog
protected ExecutionAuthenticator executionAuthenticator;
protected ThreadPoolExecutor threadPoolWithPreAuth;
// Map lowercase database names to actual remote database names for case-insensitive lookup
private Map<String, String> lowerCaseToDatabaseName = Maps.newConcurrentMap();
private volatile Map<String, String> lowerCaseToDatabaseName = Maps.newConcurrentMap();

private volatile Configuration cachedConf = null;
private byte[] confLock = new byte[0];
Expand Down Expand Up @@ -421,6 +421,9 @@ private void buildMetaCache() {
OptionalLong.of(Config.external_cache_refresh_time_minutes * 60L),
Math.max(Config.max_meta_object_cache_num, 1),
ignored -> getFilteredDatabaseNames(),
this::updateLowerCaseToDatabaseName,
(remoteName, localName) -> lowerCaseToDatabaseName.put(remoteName.toLowerCase(), remoteName),
localName -> lowerCaseToDatabaseName.remove(localName.toLowerCase()),
localDbName -> Optional.ofNullable(
buildDbForInit(null, localDbName, Util.genIdByName(name, localDbName), logType,
true)),
Expand Down Expand Up @@ -549,7 +552,6 @@ private List<Pair<String, String>> getFilteredDatabaseNames() {
Map<String, Boolean> includeDatabaseMap = getIncludeDatabaseMap();
Map<String, Boolean> excludeDatabaseMap = getExcludeDatabaseMap();

lowerCaseToDatabaseName.clear();
List<Pair<String, String>> remoteToLocalPairs = Lists.newArrayList();

allDatabases = allDatabases.stream().filter(dbName -> {
Expand All @@ -567,8 +569,6 @@ private List<Pair<String, String>> getFilteredDatabaseNames() {

for (String remoteDbName : allDatabases) {
String localDbName = fromRemoteDatabaseName(remoteDbName);
// Populate lowercase mapping for case-insensitive lookups
lowerCaseToDatabaseName.put(remoteDbName.toLowerCase(), remoteDbName);
// Apply lower_case_database_names mode to local name
int dbNameMode = getLowerCaseDatabaseNames();
if (dbNameMode == 1) {
Expand Down Expand Up @@ -610,6 +610,12 @@ private List<Pair<String, String>> getFilteredDatabaseNames() {
return remoteToLocalPairs;
}

private void updateLowerCaseToDatabaseName(List<Pair<String, String>> names) {
Map<String, String> updated = Maps.newConcurrentMap();
names.forEach(pair -> updated.put(pair.key().toLowerCase(), pair.key()));
lowerCaseToDatabaseName = updated;
}

/**
* Resets the Catalog state to uninitialized, releases resources held by {@code initLocalObjectsImpl()}
* <p>
Expand Down Expand Up @@ -1266,16 +1272,14 @@ private String getLocalDatabaseName(String dbName, boolean isReplay) {
finalName = dbName.toLowerCase();
} else if (mode == 2) {
// Mode 2: Case-insensitive comparison
finalName = lowerCaseToDatabaseName.get(dbName.toLowerCase());
if (finalName == null && !isReplay) {
// Refresh database list and try again
if (!isReplay) {
try {
getFilteredDatabaseNames();
finalName = lowerCaseToDatabaseName.get(dbName.toLowerCase());
metaCache.listNames();

@github-actions github-actions Bot Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep the routing-map clear atomic with names invalidation

resetToUninitialized() clears lowerCaseToDatabaseName under the catalog monitor, releases that monitor, and only afterward invalidates metaCache through onRefreshCache(). A concurrent getDbNullable() can reinitialize in that gap; this call then hits the still-complete old names entry without republishing the cleared map, so a following differently-cased mode-2 lookup misses. This is separate from the stale-loader thread because no load runs in this interleaving. Please order the map clear and names invalidation under the same initialization fence (or republish one atomic snapshot), and add a paused reset test.

} catch (Exception e) {
LOG.warn("Failed to refresh database list for catalog {}", getName(), e);
}
}
finalName = lowerCaseToDatabaseName.get(dbName.toLowerCase());
if (finalName == null && LOG.isDebugEnabled()) {
LOG.debug("Failed to get database name from: {}.{}, isReplay={}",
getName(), dbName, isReplay);
Expand Down
45 changes: 19 additions & 26 deletions fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public abstract class ExternalDatabase<T extends ExternalTable>
@SerializedName(value = "initialized")
protected boolean initialized = false;
// table name lower case -> table name
private Map<String, String> lowerCaseToTableName = Maps.newConcurrentMap();
private volatile Map<String, String> lowerCaseToTableName = Maps.newConcurrentMap();
@SerializedName(value = "lastUpdateTime")
protected long lastUpdateTime;
protected final InitDatabaseLog.Type dbLogType;
Expand Down Expand Up @@ -170,6 +170,9 @@ private void buildMetaCache() {
OptionalLong.of(Config.external_cache_refresh_time_minutes * 60L),
Math.max(Config.max_meta_object_cache_num, 1),
ignored -> listTableNames(),
this::updateLowerCaseToTableName,
(remoteName, localName) -> lowerCaseToTableName.put(remoteName.toLowerCase(), remoteName),
localName -> lowerCaseToTableName.remove(localName.toLowerCase()),
localTableName -> Optional.ofNullable(
buildTableForInit(null, localTableName,
Util.genIdByName(extCatalog.getName(), name, localTableName),
Expand All @@ -180,18 +183,15 @@ private void buildMetaCache() {

private List<Pair<String, String>> listTableNames() {
List<Pair<String, String>> tableNames;
lowerCaseToTableName.clear();
if (name.equals(InfoSchemaDb.DATABASE_NAME)) {
tableNames = ExternalInfoSchemaDatabase.listTableNames().stream()
.map(tableName -> {
lowerCaseToTableName.put(tableName.toLowerCase(), tableName);
return Pair.of(tableName, tableName);
})
.collect(Collectors.toList());
} else if (name.equals(MysqlDb.DATABASE_NAME)) {
tableNames = ExternalMysqlDatabase.listTableNames().stream()
.map(tableName -> {
lowerCaseToTableName.put(tableName.toLowerCase(), tableName);
return Pair.of(tableName, tableName);
})
.collect(Collectors.toList());
Expand All @@ -218,7 +218,6 @@ private List<Pair<String, String>> listTableNames() {
// Mode 2: preserve original remote case for display
localTableName = tableName;
}
lowerCaseToTableName.put(tableName.toLowerCase(), tableName);
return Pair.of(tableName, localTableName);
}).collect(Collectors.toList());
}
Expand Down Expand Up @@ -257,6 +256,12 @@ private List<Pair<String, String>> listTableNames() {
return tableNames;
}

private void updateLowerCaseToTableName(List<Pair<String, String>> names) {
Map<String, String> updated = Maps.newConcurrentMap();
names.forEach(pair -> updated.put(pair.key().toLowerCase(), pair.key()));
lowerCaseToTableName = updated;
}

public T buildTableForInit(String remoteTableName, String localTableName, long tblId,
ExternalCatalog catalog, ExternalDatabase db, boolean checkExists) {

Expand Down Expand Up @@ -432,15 +437,10 @@ public DatabaseProperty getDbProperties() {
public boolean isTableExist(String tableName) {
String remoteTblName = tableName;
if (this.isTableNamesCaseInsensitive()) {
metaCache.listNames();

@github-actions github-actions Bot Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Initialize the database before reading its names cache

On a cold object-cache miss, a database returned by ExternalCatalog.getDbNullable() is newly constructed but has not run its own makeSureInitialized(), so its metaCache is still null. CreateTableCommand.targetTableExists() immediately calls the generic DatabaseIf.isTableExist() path; with lower_case_table_names=2, this new call therefore throws before reaching the connector. Please establish database initialization (and handle initialization failure) before using metaCache, and cover a cold mode-2 existence probe.

remoteTblName = lowerCaseToTableName.get(tableName.toLowerCase());
if (remoteTblName == null) {
// Here we need to execute listTableNames() once to fill in lowerCaseToTableName
// to prevent lowerCaseToTableName from being empty in some cases
listTableNames();
remoteTblName = lowerCaseToTableName.get(tableName.toLowerCase());
if (remoteTblName == null) {
return false;
}
return false;
}
}
return extCatalog.tableExist(ConnectContext.get().getSessionContext(), remoteName, remoteTblName);
Expand Down Expand Up @@ -509,26 +509,21 @@ private String getLocalTableName(String tableName, boolean isReplay) {
finalName = tableName.toLowerCase();
}
if (this.isTableNamesCaseInsensitive()) {
if (!isReplay) {
metaCache.listNames();

@github-actions github-actions Bot Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Use the known event entry before requiring a full listing

On a cold mode-2 cache, registerTable() has already installed the table object and its lowercase routing entry, but HMSExternalDatabase.registerTable() immediately calls getTableNullable() to apply the event update time. This unconditional listNames() sees the incomplete entry and forces a full remote enumeration first; if that enumeration fails, the authoritative event itself fails even though the requested table is already cached. The base path consulted the specific routing entry before listing. Please allow a known incremental hit here and enumerate only on a miss, with a cold registration/loader-failure test.

}
finalName = lowerCaseToTableName.get(tableName.toLowerCase());
if (finalName == null) {
if (isReplay) {
if (LOG.isDebugEnabled()) {
if (LOG.isDebugEnabled()) {
if (isReplay) {
LOG.debug("failed to get final table name from: {}.{}.{}, is replay = true",
getCatalog().getName(), getFullName(), tableName);
}
return null;
}
// Here we need to execute listTableNames() once to fill in lowerCaseToTableName
// to prevent lowerCaseToTableName from being empty in some cases
listTableNames();
finalName = lowerCaseToTableName.get(tableName.toLowerCase());
if (finalName == null) {
if (LOG.isDebugEnabled()) {
} else {
LOG.debug("failed to get final table name from: {}.{}.{}",
getCatalog().getName(), getFullName(), tableName);
}
return null;
}
return null;
}
}
if (LOG.isDebugEnabled()) {
Expand Down Expand Up @@ -579,7 +574,6 @@ public void unregisterTable(String tableName) {
if (isInitialized()) {
metaCache.invalidate(dorisTable.getName(),
Util.genIdByName(extCatalog.getName(), name, dorisTable.getName()));
lowerCaseToTableName.remove(dorisTable.getName().toLowerCase());
}

Env.getCurrentEnv().getExtMetaCacheMgr().invalidateTableCache(dorisTable);
Expand All @@ -602,7 +596,6 @@ public boolean registerTable(TableIf tableIf) {
String localName = extCatalog.fromRemoteTableName(this.remoteName, tableName);
metaCache.updateCache(tableName, localName, (T) tableIf,
Util.genIdByName(extCatalog.getName(), name, localName));
lowerCaseToTableName.put(tableName.toLowerCase(), tableName);
}
setLastUpdateTime(System.currentTimeMillis());
return true;
Expand Down
8 changes: 7 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/LegacyMetaCacheFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import java.util.Optional;
import java.util.OptionalLong;
import java.util.concurrent.ExecutorService;
import java.util.function.BiConsumer;
import java.util.function.Consumer;

/**
* Bridge factory for legacy {@link MetaCache} users.
Expand All @@ -40,10 +42,14 @@ public LegacyMetaCacheFactory(ExecutorService refreshExecutor) {
public <T> MetaCache<T> build(String name,
OptionalLong expireAfterAccessSec, OptionalLong refreshAfterWriteSec, long maxSize,
CacheLoader<String, List<Pair<String, String>>> namesCacheLoader,
Consumer<List<Pair<String, String>>> namesCacheUpdateAction,
BiConsumer<String, String> nameUpdateAction,
Consumer<String> nameInvalidationAction,
CacheLoader<String, Optional<T>> metaObjCacheLoader,
RemovalListener<String, Optional<T>> removalListener) {
return new MetaCache<>(
name, refreshExecutor, expireAfterAccessSec, refreshAfterWriteSec,
maxSize, namesCacheLoader, metaObjCacheLoader, removalListener);
maxSize, namesCacheLoader, namesCacheUpdateAction, nameUpdateAction, nameInvalidationAction,
metaObjCacheLoader, removalListener);
}
}
Loading
Loading