github-actions[bot] commented on code in PR #68196:
URL: https://github.com/apache/doris/pull/68196#discussion_r4090804402
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -619,15 +635,29 @@ 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) {
+ // 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);
Review Comment:
[P1] Retire hidden table objects when the name mapping is gone. A names
refresh can remove the mode-2 `lowerCaseToTableName` entry after the remote
DROP without evicting the old object from `MetaCache.metaObjCache`.
`getTableForReplay` then reaches this miss solely because the mapping is
absent, and `invalidateTableByNameOrWider` repeats that miss and clears
engine/row-count state only; if the name is recreated, `getTableNullable` can
resolve it again and return the prior initialized table object. The Paimon drop
path already handles this predicate with `!hasLocalTableName(...)` plus
`retireAllTableObjectsWithoutEngineInvalidation()`, but the generic path used
by Hive does not. Retire the table-object generation for an unresolved name
before returning, and add a mode-2 DROP/recreate regression with the old object
still resident.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -941,7 +947,7 @@ public Optional<ExternalDatabase<? extends ExternalTable>>
getDbForReplay(String
LOG.debug("getDbForReplay from metacache, db: {}.{}, catalog id:
{}, is catalog init: {}",
this.name, dbName, this.id, isInitialized());
}
- if (!isInitialized()) {
+ if (!isInitialized() || metaCache == null) {
Review Comment:
[P1] Preserve an unresolved-name signal for DROP TABLE cleanup. A mode-2
names refresh can remove `lowerCaseToDatabaseName` while the canonical database
object remains in `metaObjCache`; falling back to the event spelling here then
makes `tryGetMetaObj` return empty. Hive, Iceberg, and MaxCompute
`afterDropTable` treat that as nothing to do, so both live post-drop cleanup
and follower replay bypass the new table fallback and leave the old
database/table objects, engine entries, and row count reusable after the name
reappears. Paimon already handles this empty-DB state explicitly. Have the
generic hooks retire hidden database objects and publish a failure-safe catalog
invalidation, and add a mode-2 DROP/recreate regression with the old generation
primed.
##########
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);
+ return;
+ }
+ Optional<? extends ExternalTable> table =
db.get().getTableForReplay(tableName);
+ if (table.isPresent()) {
+ invalidateTableCache(table.get());
+ } else {
+ invalidateDb(catalogId, db.get().getId(), db.get().getFullName());
+ }
+ }
+
+ public void invalidateRowCountCache(ExternalTable table) {
+ rowCountCache.invalidateTable(table.getCatalog().getId(),
table.getDb().getId(), table.getId());
+ }
+
+ public void invalidateRowCountCache(long catalogId) {
Review Comment:
[P1] Use this catalog fence on Paimon's unresolved-name retirements. Both
the lost-DB-mapping `afterDropTable` branch and `afterDropDbNoOp` call
`retireAllDatabaseObjectsWithoutEngineInvalidation()`, which makes the changed
removal listener run `resetMetaToUninitialized(false, false)` and suppress
every per-DB row-count fence. The former then clears only the Paimon engine
group and the latter intentionally performs no engine flush, so neither path
reaches this new shared catalog fence. A same-name recreation can therefore
reuse the deterministic table id and its stale count. Publish this fence in a
failure-safe boundary independent of the engine flush, and extend both
lost-mapping regressions to prime and assert row-count state.
--
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]
