924060929 commented on code in PR #64160:
URL: https://github.com/apache/doris/pull/64160#discussion_r3746412002
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1452,12 +1453,19 @@ protected void invalidateDatabaseCache(String
localDbName) {
databaseNames.compute("", (ignored, current) ->
current == null ? null :
current.withoutLocalName(localDbName));
}
- if (databases != null) {
- databases.invalidateKeyAndRun(
- localDbName,
- () -> dbIdNameIndex.removeName(localDbName));
+ Pair<ExternalDatabase<? extends ExternalTable>, Long> invalidation =
databases == null
+ ? Pair.of(null, dbIdNameIndex.removeName(localDbName))
+ : databases.invalidateKeyAndGet(
+ localDbName,
+ removedDb -> Pair.of(removedDb,
dbIdNameIndex.removeName(localDbName)));
+ if (invalidation.first != null) {
Review Comment:
**[P1] Do not skip database-scope invalidation when the removed DB object is
hot.**
The normal `PluginDrivenExternalCatalog.dropDb()` path first calls
`getDbNullable(dbName)`, so the database object is hot, then calls
`unregisterDatabase()`. In that case `invalidateKeyAndGet()` returns a non-null
`removedDb` and this branch returns before `cacheMgr.invalidateDb(...)`.
Consequently the row-count cache, engine metadata caches, and database-scope
sorted-partition cache are not invalidated. This also regresses the base
behavior, where `unregisterDatabase()` performed metadata invalidation
unconditionally after removing the object cache entry. A same-name recreate can
therefore retain the old row count and FE metadata.
Please keep the object/index removal atomic, but after it completes use the
removed index ID to call `invalidateDb(...)` for both hot and cold object
entries. Since a hot object should have a retained ID mapping, assert that
invariant instead of silently returning. Please also add a hot-path test; the
current `testUnregisterColdDatabaseInvalidatesRowCountByRetainedDbId`
explicitly evicts the object first and therefore cannot catch this branch.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -175,42 +179,57 @@ public void replayRefreshTable(ExternalObjectLog log) {
return;
}
boolean hasDbName = !Strings.isNullOrEmpty(log.getDbName());
+ boolean hasTableName = !Strings.isNullOrEmpty(log.getTableName());
+ boolean isRename = !Strings.isNullOrEmpty(log.getNewTableName());
String localDbName = hasDbName
? log.getDbName()
: catalog.getDbNameForReplay(log.getDbId()).orElse(null);
Optional<ExternalDatabase<? extends ExternalTable>> db =
localDbName == null ? Optional.empty() :
catalog.getDbForReplay(localDbName);
+ // Rename logs carry names but no IDs. Rebuild the old table identity
used by the row-count cache.
+ long dbIdForInvalidation = isRename && hasDbName
Review Comment:
**[P1] Preserve row-count invalidation for pre-upgrade name-only refresh
logs.**
Before this PR, `createForRefreshDb/Table` persisted names but did not
populate `dbId/tableId`. During a rolling upgrade, a new follower can therefore
replay an old leader's log with names present and both new `long` fields
defaulting to `0`. This code reconstructs deterministic IDs only for rename
logs; an ordinary refresh keeps `(dbIdForInvalidation, tableIdForInvalidation)
== (0, 0)`. If the DB/table object cache is cold while the independent
row-count cache is still hot, the cold replay branches invalidate `(catalogId,
0, 0)` and miss the real entry. `replayRefreshDb()` has the analogous `dbId ==
0` gap.
Please recover IDs for legacy name-bearing refresh DB/table logs as well
(without forcing remote metadata loading), or conservatively invalidate a wider
row-count scope when an exact ID cannot be recovered. Add compatibility tests
using a genuinely old-format/name-only log with a cold object cache and hot
row-count entry; the current tests use the new constructors with IDs and only
cover new-leader to new-follower replay.
--
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]
