github-actions[bot] commented on code in PR #68196:
URL: https://github.com/apache/doris/pull/68196#discussion_r4121216604
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -619,15 +640,36 @@ public boolean unregisterTableForReplay(String tableName)
{
// Check whether the table still exists in the local replay cache.
ExternalTable dorisTable = getTableForReplay(tableName).orElse(null);
if (dorisTable == null) {
+ // A mode-2 table-name mapping can disappear while the old object
stays resident in
+ // MetaCache.metaObjCache; getTableForReplay then misses only
because the mapping is
+ // absent. Retire the hidden table-object generation so a
same-name recreation cannot
+ // reuse the prior incarnation, then widen the engine and
row-count fence.
+ if (!hasLocalTableName(tableName)) {
+ retireAllTableObjectsWithoutEngineInvalidation();
Review Comment:
[P1] Fence row counts before retiring this unresolved table generation. With
a lost mode-2 name mapping, this helper swaps in a fresh table-object cache
before `invalidateTableByNameOrWider` reaches its DB row-count fence, after
routed engine work. Cold `replayRefreshTable` calls the same helper before its
own DB fence. A concurrent names refresh after DROP/recreate can load the
replacement under the same deterministic ID and consume the old completed
count. Put a DB-scope opening fence in the shared retirement helper or at both
call sites, retain their completion fences, and latch the swap in a regression.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1247,10 +1273,38 @@ 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;
+ }
+ long dbId = Util.genIdByName(name, localDbName);
+ metaCache.invalidate(localDbName, dbId);
Review Comment:
[P1] Fence a cold database's row counts before releasing its name slot. When
the DB object was evicted but its table count remains cached,
`metaCache.invalidate` removes the local name/ID without a removal callback.
The following `invalidateDb` runs engine cleanup before its row-count fence, so
a same-name remote recreation can load a new table with the same deterministic
ID and read the old completed count in that gap. Publish an opening DB fence
before `metaCache.invalidate`, retain the completion fence, and test a cold
DROP/recreate interleaving.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1268,6 +1322,43 @@ public void
retireAllDatabaseObjectsWithoutEngineInvalidation() {
metaCache.invalidateObjects();
} finally {
invalidateEngineCacheOnDatabaseRemoval.remove();
+ // The removal callbacks above suppress every per-database
row-count fence, so publish
+ // one catalog-wide fence here; otherwise a recreated same-name
object can reuse a stale
+ // count through its deterministic table id.
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(getId());
Review Comment:
[P1] Publish the catalog row-count fence before swapping this unresolved
database generation. `metaCache.invalidateObjects()` exposes the new DB-object
cache before the new `finally` fence runs, and its removal callbacks suppress
every per-DB fence. After a mode-2 mapping loss and same-name DROP/recreate, a
concurrent query can load the replacement and use the old completed table count
before this fence. Add an opening catalog fence before `invalidateObjects`,
retain this completion fence, and cover the swap with a latch test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalMetaCache.java:
##########
@@ -150,7 +150,10 @@ public void invalidateTable(ExternalTable table) {
@Override
public void invalidateDb(long catalogId, String dbName) {
try {
- invalidatePaimonTables(catalogId, nameMapping ->
matchDb(nameMapping, dbName));
+ // The SDK cache can be populated without any Doris table entry
(for example by a
+ // metadata TVF). A name-only database invalidation cannot recover
the remote name
+ // from an evicted database object, so fence the SDK catalog
independently.
+ tableLoader.invalidateCatalog(catalogId);
Review Comment:
[P2] Keep Paimon SDK invalidation scoped to the affected database. This
name-based route now calls `invalidatePaimonCatalog`, which evicts every
database's SDK table and partition entries. It also runs after the typed,
database-scoped removal callback on an ordinary warm `DROP DATABASE`, adding a
redundant second SDK flush that clears unrelated hot databases. Cold replay
with a retained DB identity takes the same wide route. Preserve the SDK-only
cold-cache coverage, but use the remote DB identity when known and avoid the
second catalog flush after a typed invalidation; add a two-DB regression with
an unrelated SDK entry primed.
--
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]
