github-actions[bot] commented on code in PR #68196:
URL: https://github.com/apache/doris/pull/68196#discussion_r4080096953
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -593,15 +604,22 @@ public void unregisterTable(String tableName) {
// check if the table exists in cache, it not, does return
ExternalTable dorisTable = getTableForReplay(tableName).orElse(null);
if (dorisTable == null) {
+ // The table object cache is much smaller than the row-count and
Hive engine caches. A drop
+ // or rename must still retire stale entries when the table object
was evicted, and the
+ // event carries the caller spelling, so widen to the canonical
scope.
+ Env.getCurrentEnv().getExtMetaCacheMgr()
+ .invalidateTableByNameOrWider(extCatalog.getId(),
getFullName(), tableName);
return;
}
+ // Fence the held table before releasing its local name/id slot: a
query does not take this
+ // database's write lock, so a same-name replacement admitted right
after the removal could
+ // otherwise observe the retired row-count entry through the
deterministic table id.
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateTableCache(dorisTable);
Review Comment:
[P1] Keep local table retirement independent of routed invalidation failure.
Live `dropTableImpl` has already committed remotely before `afterDropTable`
reaches this call, but `safeInvalidate` lets an engine `invalidateTable`
exception propagate; because `metaCache.invalidate` now runs afterward, the
retired table remains locally visible and `ExternalCatalog.dropTable` also
skips its `DropInfo` edit log. The database path already uses a failure-safe
local-removal boundary for the same reason. Preserve the pre-admission
row-count fence, make local name/id/object removal unconditional, then perform
routed invalidation plus completion fencing; add throwing-engine
DROP/rename/replay tests.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -233,6 +261,10 @@ public void refreshExternalTableFromEvent(String
catalogName, String dbName, Str
if (!(catalog instanceof ExternalCatalog)) {
throw new DdlException("Only support refresh ExternalCatalog
Tables");
}
+ // Whole-table events are already committed remotely. Fence the cache
by cached identity
+ // before any database/table reload can fail and make the not-found
path return.
+ Env.getCurrentEnv().getExtMetaCacheMgr()
+ .invalidateRowCountCache(catalog.getId(), dbName, tableName);
Review Comment:
[P1] Retire engine caches on this cold event path too. This new pre-lookup
call fences only row counts; if the bounded DB/table object cache is cold and
the fallible HMS rebuild returns null, this handler returns normally and the
event processor advances `lastSyncedEventId`, while independently resident Hive
schema/partition/file entries remain reusable. The same row-count-only early
return exists in `refreshPartitions` and
`CatalogMgr.add/dropExternalPartitions`. This is distinct from the existing
row-count threads and the cold follower-refresh replay thread. Route a
canonical table/DB/catalog engine invalidation before acknowledging these
misses, and add primed-engine-cache cold-miss tests for the whole-table and
partition event families.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java:
##########
@@ -433,9 +438,16 @@ public void removeCatalog(long catalogId) {
* retained runtime, its group) and lets the next statement load a
coherent generation.
*/
public void onCatalogOperationalContextChanged(long catalogId) {
- routeCatalogEngines(catalogId, cache -> safeInvalidate(
- cache, catalogId, "onCatalogOperationalContextChanged",
- () -> cache.invalidateCatalogEntries(catalogId)));
+ try {
+ routeCatalogEngines(catalogId, cache -> safeInvalidate(
Review Comment:
[P1] Make this committed retirement reachable when reset cleanup fails. The
only production caller invokes it after
`externalCatalog.modifyCatalogProps(newProps)`, but that call first publishes
the new properties and then runs `resetToUninitialized(false)`; JDBC
`closeClient`, Trino `connector.shutdown`, and other `onClose` work can throw.
That skips both this engine/row-count retirement and the subsequent
`OP_ALTER_CATALOG_PROPS` log, leaving the leader on undurable new properties
while followers retain the old target and cold old-target entries remain
reusable under the same IDs. The rollback path above is failure-safe, but the
committed path is not. Make publication/reset/retirement/durability one
failure-safe transition and add a successful-ALTER test with throwing cleanup.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -114,6 +114,10 @@ public void replayRefreshDb(ExternalObjectLog log) {
}
if (!db.isPresent()) {
+ // The database object cache can be cold while row-count entries
from an earlier
+ // generation are still resident. Retire the catalog scope because
replay cannot
+ // recover a canonical database id without loading remote metadata.
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(catalog.getId());
Review Comment:
[P1] Retire engine entries on cold REFRESH DATABASE replay too. This miss
branch now clears only row counts, whereas the warm `refreshDbInternal` path
routes DB invalidation; an evicted DB object can coexist with independently
resident Hive schema/partition/file entries, so replay completes and rebuilt
objects can reuse pre-refresh metadata. This is the database-level counterpart
of, but a distinct location from, the cold table-refresh replay thread. Route
canonical DB invalidation when possible (or conservatively invalidate the
catalog) and add a cold follower test with real engine entries 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]
