wenzhenghu commented on code in PR #65126:
URL: https://github.com/apache/doris/pull/65126#discussion_r3689696775
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -599,10 +601,154 @@ private String getLocalTableName(String tableName,
boolean isReplay) {
return finalName;
}
+ private NameCacheValue getTableNamesValue(boolean allowLoad) {
+ if (tableNames == null) {
+ return null;
+ }
+ return allowLoad ? tableNames.get("") : tableNames.getIfPresent("");
+ }
+
+ // Centralize names-negative-lookup handling so all table lookup paths
share the same config-driven policy.
+ @Nullable
+ private <R> R resolveTableNameFromSnapshot(String lookupName, boolean
isReplay,
+ Function<NameCacheValue, R> resolver) {
+ NameCacheValue cached = getTableNamesValue(false);
+ if (cached == null) {
+ if (isReplay) {
+ return null;
+ }
+ NameCacheValue loaded = getTableNamesValue(true);
+ return loaded == null ? null : resolver.apply(loaded);
+ }
+ R resolved = resolver.apply(cached);
+ if (resolved != null || isReplay ||
!Config.enable_external_meta_cache_name_miss_refresh) {
+ return resolved;
+ }
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("refresh table names after hot-snapshot miss, catalog:
{}, db: {}, lookup: {}",
+ getCatalog().getName(), getFullName(), lookupName);
+ }
+ resetMetaCacheNames();
+ NameCacheValue refreshed = getTableNamesValue(true);
+ return refreshed == null ? null : resolver.apply(refreshed);
+ }
+
+ private List<String> listLocalTableNamesFromCache() {
+ NameCacheValue namesValue = java.util.Objects.requireNonNull(
+ getTableNamesValue(true), "table names cache can not be null");
+ return namesValue.localNames();
+ }
+
+ private List<String> listLocalTableNamesWithoutCache(SessionContext
sessionContext) {
+ return
listTableNames(sessionContext).stream().map(Pair::value).collect(Collectors.toList());
+ }
+
+ @Nullable
+ private String getRemoteTableName(String localTableName, boolean isReplay)
{
+ // Route local-to-remote resolution through the shared helper so miss
reload stays consistent with lookups.
+ return resolveTableNameFromSnapshot(localTableName, isReplay,
+ namesValue ->
namesValue.remoteNameOfLocalName(localTableName));
+ }
+
+ private Optional<Pair<String, String>>
findTableNamePairWithoutCache(SessionContext sessionContext,
+ String requestedTableName) {
+ return listTableNames(sessionContext).stream()
+ .filter(pair -> matchesLocalTableName(pair.value(),
requestedTableName))
+ .findFirst();
+ }
+
+ private boolean matchesLocalTableName(String localTableName, String
requestedTableName) {
+ if (isStoredTableNamesLowerCase()) {
+ return
localTableName.equals(requestedTableName.toLowerCase(Locale.ROOT));
+ }
+ if (isTableNamesCaseInsensitive()) {
+ return localTableName.equalsIgnoreCase(requestedTableName);
+ }
+ return localTableName.equals(requestedTableName);
+ }
+
+ private String resolveTableNameForInvalidation(String tableName) {
+ if (isStoredTableNamesLowerCase()) {
+ return tableName.toLowerCase(Locale.ROOT);
+ }
+ if (!isTableNamesCaseInsensitive()) {
+ return tableName;
+ }
+
+ String localTableName = getLocalTableName(tableName, true);
+ if (localTableName != null) {
+ return localTableName;
+ }
+ T cachedTable = tables.findIfPresent(key ->
key.equalsIgnoreCase(tableName));
+ if (cachedTable != null) {
+ return cachedTable.getName();
+ }
+ return tableIdToName.values().stream()
+ .filter(name -> name.equalsIgnoreCase(tableName))
+ .findFirst()
+ .orElse(tableName);
+ }
+
+ private void updateTableCache(T table, String remoteTableName, String
localTableName) {
+ updateTableCache(table, remoteTableName, localTableName, false);
+ }
+
+ protected void updateTableCache(T table, String remoteTableName, String
localTableName,
+ boolean forceUpdateCacheState) {
+ buildMetaCache();
+ // Runtime incremental events only maintain names and object entries
that are already hot. The ID map is a
+ // lightweight lookup index and must always track registered objects
so normal by-ID lookup can load on demand.
+ // By default, incremental updates keep cold names/object cache
entries cold.
+ // forceUpdateCacheState is only for paths that intentionally populate
those cold entries.
+ if (forceUpdateCacheState) {
+ tableNames.compute("", (ignored, current) ->
+ (current == null ? NameCacheValue.empty() :
current).withName(remoteTableName, localTableName));
+ } else {
+ // Keep a cold names entry cold, but still advance its generation
so an in-flight pre-event load cannot
+ // publish a stale snapshot after this incremental update.
+ tableNames.compute("", (ignored, current) ->
+ current == null ? null : current.withName(remoteTableName,
localTableName));
+ }
+ tables.computeAndRun(
+ localTableName,
+ (ignored, current) -> (forceUpdateCacheState || current !=
null) ? table : null,
+ () -> tableIdToName.put(table.getId(), localTableName));
+ }
+
+ protected void invalidateTableCache(String localTableName) {
+ if (tableNames != null) {
+ // Keep a cold names entry cold, but fence any in-flight pre-drop
load from publishing stale state.
+ tableNames.compute("", (ignored, current) ->
+ current == null ? null :
current.withoutLocalName(localTableName));
+ }
+ if (tables != null) {
+ tables.invalidateKeyAndRun(
+ localTableName,
+ () -> tableIdToName.entrySet().removeIf(
+ entry -> entry.getValue().equals(localTableName)));
Review Comment:
This follow-up has now been implemented in commit `8fe375d3275`.
The retained database/table ID mappings are now maintained by a strict
bidirectional `IdNameIndex`. Targeted cleanup uses the reverse
canonical-local-name-to-ID direction, so `removeName()` removes the exact pair
in O(1) instead of scanning the entire retained ID map under the
database/publication locks.
The change also keeps the index ordered with `MetaCacheEntry`
publication/invalidation and rejects conflicting ID/name pairs before cache
mutation. Database event IDs are canonicalized first so the one-name/one-ID
invariant is valid across HMS events and normal full loads.
Focused validation passed for `IdNameIndexTest`, `ExternalDatabaseTest`,
`ExternalCatalogTest`, and `RefreshManagerTest` (95 tests, 0 failures/errors),
together with FE checkstyle and `git diff --check`.
This resolves the retained ID-map O(N) cleanup concern raised in this
thread. The broader issue #65779 should remain open for its other independent
items, including full `NameCacheValue` snapshot rebuild costs and the remaining
extreme-cold fallback scan.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
