github-actions[bot] commented on code in PR #69458:
URL: https://github.com/apache/doris/pull/69458#discussion_r4346749491
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -133,7 +137,11 @@ private void refreshDbInternal(ExternalDatabase db) {
}
} finally {
try {
- db.resetMetaToUninitialized();
+ try {
+ db.resetMetaToUninitialized();
+ } finally {
+ ((ExternalCatalog)
db.getCatalog()).invalidateDatabaseLookup(db.getFullName());
Review Comment:
[P2] Update the existing refresh tests for replacement objects
Lookup eviction changes the object lifetime, but
`RefreshDbTest.testRefreshDatabase()` still retains `db1` across refresh and
asserts that its `lastUpdateTime` later increases. That database is now
permanently retired, so subsequent live reads leave its timestamp unchanged and
the `l4 > l3` assertion fails. Likewise, `RefreshTableTest.testRefreshTable()`
retains the evicted table and expects `getFullSchema()` to advance its
timestamp; the schema loader now resolves and updates the replacement table
instead. Update both existing tests to reacquire the current objects and assert
the intended replacement behavior so the FE suite remains compatible with this
change.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -959,9 +1038,16 @@ public void setLastUpdateTime(long lastUpdateTime) {
@Override
public final void onClose() {
+ synchronized (this) {
+ feCacheRetired = true;
Review Comment:
[P1] Allow renamed catalogs to initialize a new cache owner
`CatalogMgr.alterCatalogName()` calls `cleanupRemovedCatalog()` (and
therefore `onClose()`), then renames and re-registers the same catalog
instance. Setting this flag permanently means `ALTER CATALOG old RENAME new`
succeeds, but every subsequent metadata access fails in `getFeMetaCacheOwner()`
with "External catalog cache owner has been retired". `resetToUninitialized()`
never clears the flag, and `replayAlterCatalogName()` reuses the instance in
the same way, so followers are affected too. Distinguish terminal removal from
rename cleanup, or explicitly reactivate/recreate the owner when re-registering
the catalog; cover rename and its replay.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -789,7 +836,9 @@ public void unregisterTable(String tableName) {
setLastUpdateTime(System.currentTimeMillis());
localTableName = resolveTableNameForInvalidation(tableName);
// Always clean local names/object/ID state, even when the object
entry is cold or already evicted.
- invalidateTableCache(localTableName);
+ if (!cacheEntriesReleased) {
Review Comment:
[P2] Skip cache-based name resolution for retired databases
With `external_cache_expire_time_seconds_after_access=0` and
`lower_case_table_names=2`, the database lookup returns an uncached database
and its new discard callback retires it before `tables` is initialized. An HMS
DROP/RENAME event reaches `unregisterTable()`, but `makeSureInitialized()` now
returns immediately for that database and `resolveTableNameForInvalidation()`
still runs before this guard. Its case-insensitive fallback dereferences
`tables.findIfPresent()` on null. Event application then fails and the sync
driver resets its cursor for a full resync. Include name resolution in the
retired-entry handling (or provide a transient resolver), and cover this
disabled-cache configuration.
--
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]
