924060929 commented on code in PR #68196:
URL: https://github.com/apache/doris/pull/68196#discussion_r4121047713
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonMetadataOps.java:
##########
@@ -408,12 +408,9 @@ public void afterDropTable(String dbName, String tblName) {
invalidatePaimonCatalogForUnresolvedReplay();
}
} else {
- // The database itself could not be resolved (for example a
mode-2 mapping was lost
- // before replay). Retire any retained legacy database object
first so a same-name
- // recreation cannot reuse its stale nested table-name cache,
then flush the engine
- // group; a failure in either is best-effort so the drop log
is still written.
-
dorisCatalog.retireAllDatabaseObjectsWithoutEngineInvalidation();
- invalidatePaimonCatalogForUnresolvedReplay();
+ // A cold DB with a retained canonical mapping has a narrow
invalidation target.
+ // Only a genuinely lost mapping requires catalog-wide
hidden-object retirement.
+ dorisCatalog.invalidateColdDatabaseForReplay(dbName);
Review Comment:
Fixed in 537ea2cc0cb. Paimon's name-based DB invalidation now fences its SDK
cache independently of Doris table entries. The existing SDK-only replay
REFRESH/DROP tests pass.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -146,8 +150,20 @@ public void resetMetaToUninitialized(boolean
invalidateEngineCache) {
objectInvalidation.run();
}
}
- if (invalidateEngineCache) {
- Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(this);
+ try {
+ if (invalidateEngineCache) {
+ // Route through the typed overload: connector-specific caches
(for example Paimon's
+ // table loader) are keyed by the database object and are not
fully covered by the
+ // name-based scan in invalidateDb(long, String).
+ Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(this);
+ }
+ } finally {
+ if (invalidateRowCountCache) {
+ // Independent of the routed invalidation: a connector cache
failure (for example
+ // Paimon's CacheException) must not skip the row-count fence.
+ Env.getCurrentEnv().getExtMetaCacheMgr()
+ .invalidateRowCountCache(extCatalog.getId(), getId());
Review Comment:
Fixed in 537ea2cc0cb with a latch regression test in f1b09556eef. Database
reset fences row counts before retiring the table-object generation, and the
test pauses after the swap but before routed invalidation/final fencing to
assert the opening fence already ran.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1247,10 +1273,37 @@ public void unregisterDatabase(String dbName) {
if (LOG.isDebugEnabled()) {
LOG.debug("unregister database [{}]", dbName);
}
- if (isInitialized()) {
- metaCache.invalidate(dbName, Util.genIdByName(name, dbName));
+ if (!isInitialized()) {
+ Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(getId(),
dbName);
+ return;
}
- Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(getId(), dbName);
+ String localDbName = getLocalDatabaseName(dbName, true);
+ if (localDbName == null) {
+ // A mode-2 remote-to-local mapping can disappear (for example
after a names refresh)
+ // while the resident database object survives. The canonical key
is then unknown, so
+ // treat the scope as unknown: retire every cached database object
and flush the engine
+ // caches and row counts catalog-wide instead of evicting the
wrong local key.
+ retireAllDatabaseObjectsWithoutEngineInvalidation();
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateCatalog(getId());
+ return;
+ }
+ metaCache.invalidate(localDbName, Util.genIdByName(name, localDbName));
+ Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(getId(),
localDbName);
Review Comment:
Fixed in 537ea2cc0cb. `unregisterDatabase` now computes and carries the
canonical DB ID through `metaCache.invalidate` into the explicit-ID manager
overload, so unrelated row counts are not invalidated.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java:
##########
@@ -883,15 +892,33 @@ private void
alterExternalCatalogPropsFenced(ExternalCatalog externalCatalog, Ca
Integer[] sec = {metadataRefreshIntervalSec,
metadataRefreshIntervalSec};
Env.getCurrentEnv().getRefreshManager().addToRefreshMap(catalogId,
sec);
}
- externalCatalog.modifyCatalogProps(newProps);
// The commit reset the catalog's execution context and closed its SDK
resources. Cached
// base generations and projections are bound to the replaced context;
retire them now so
// the next statement loads a generation the planning fences accept,
instead of retrying
- // against an unplannable cached generation until managed refresh.
+ // against an unplannable cached generation until managed refresh. The
properties are
+ // published before the reset's throwable cleanup, so retirement must
run either way.
Env currentEnv = Env.getCurrentEnv();
ExternalMetaCacheMgr cacheMgr = currentEnv == null ? null :
currentEnv.getExtMetaCacheMgr();
- if (cacheMgr != null) {
-
cacheMgr.onCatalogOperationalContextChanged(externalCatalog.getId());
+ try {
+ externalCatalog.modifyCatalogProps(newProps);
+ } catch (RuntimeException e) {
+ if (!isReplay) {
+ throw e;
+ }
+ // A follower must not terminate because a local connector cleanup
failed while applying
+ // an already-durable ALTER record. The property publication and
the derived-state
+ // transitions above are failure-safe, so the record is considered
applied.
+ LOG.warn("Failed to complete local cleanup while replaying ALTER
CATALOG for {}: {}",
+ externalCatalog.getName(), e.getMessage(), e);
+ } finally {
+ if (cacheMgr != null) {
+
cacheMgr.onCatalogOperationalContextChanged(externalCatalog.getId());
Review Comment:
Fixed in 537ea2cc0cb. ALTER CATALOG now fences catalog row counts before
`modifyCatalogProps`, retains the completion fence, and tests the call order
across property publication.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java:
##########
@@ -691,15 +785,55 @@ public void invalidateTableCache(ExternalTable
dorisTable) {
long catalogId = dorisTable.getCatalog().getId();
// Typed table invalidation bypasses the name-based invalidateTable()
entry point, so the
// Lance access-cache retirement that used to happen there has to be
repeated here.
- invalidateLanceTableAccess(catalogId);
- routeCatalogEngines(catalogId, cache -> safeInvalidate(
- cache, catalogId, "invalidateTable", () ->
cache.invalidateTable(dorisTable)));
+ try {
+ invalidateLanceTableAccess(catalogId);
+ routeCatalogEngines(catalogId, cache -> safeInvalidate(
+ cache, catalogId, "invalidateTable", () ->
cache.invalidateTable(dorisTable)));
+ } finally {
+ invalidateRowCountCache(dorisTable);
+ }
if (LOG.isDebugEnabled()) {
LOG.debug("invalid table cache for {}.{} in catalog {}",
dorisTable.getRemoteDbName(),
dorisTable.getRemoteName(),
dorisTable.getCatalog().getName());
}
}
+ /**
+ * Best-effort invalidation for a metadata event that carries the caller's
DB/table spelling.
+ * Resolves canonical local identity, then fences engine caches and row
counts at the narrowest
+ * scope that still covers the event; widens to the canonical database or
catalog scope when the
+ * cached object has already been evicted, so caller spelling can never
miss a canonical key.
+ */
+ public void invalidateTableByNameOrWider(long catalogId, String dbName,
String tableName) {
+ Optional<ExternalDatabase<? extends ExternalTable>> db =
getCachedDb(catalogId, dbName);
+ if (!db.isPresent()) {
+ invalidateCatalog(catalogId);
Review Comment:
Fixed in f1b09556eef (implementation in 537ea2cc0cb). Cold event
invalidation now resolves the retained canonical database name/ID and fences
that DB; catalog scope remains only when identity is unknown. Added a cold-DB
routing/row-count scope test.
--
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]
