924060929 commented on code in PR #68238:
URL: https://github.com/apache/doris/pull/68238#discussion_r4061242892
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -128,7 +128,7 @@ public void resetMetaToUninitialized() {
metaCache.invalidateAll();
}
}
-
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(extCatalog.getId(),
getFullName());
+ Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(this);
Review Comment:
Addressed in the current head.
ExternalCatalog.refreshMetaCacheOnly(invalidCache) sets a thread-local that
suppresses per-database engine invalidation while the catalog-wide cache is
being retired, so only the single catalog-wide scan runs. Covered by
testCatalogRefreshSkipsPerDatabaseEngineInvalidation.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java:
##########
@@ -125,11 +132,68 @@ public Table getPaimonTable(NameMapping nameMapping) {
}
public Table getPaimonTable(NameMapping nameMapping, String branch, String
queryType) {
- makeSureInitialized();
Identifier identifier = tableIdentifier(nameMapping, branch,
queryType);
return loadPaimonTable(nameMapping, queryType, identifier);
}
+ public synchronized void invalidatePaimonTable(NameMapping nameMapping)
throws Exception {
+ // Property changes reset and close the SDK catalog before retiring
Doris cache entries.
+ // Do not recreate that catalog merely to invalidate an already
retired generation.
+ if (!isInitialized()) {
+ return;
+ }
+ Identifier identifier = tableIdentifier(nameMapping, null, null);
+ withSdkCatalogCacheWriteLock(() -> executionAuthenticator.execute(()
-> {
+ catalog.invalidateTable(identifier);
Review Comment:
Addressed in the current head.
invalidateCachedPaimonTables/invalidatePaimonTableEntries match cached
identifiers case-insensitively via tableMatches/identifierPartEquals using
catalog.caseSensitive(), and tableLockKey normalizes case so the same logical
table maps to one striped lock. Covered by
testWrappedCaseInsensitiveSdkCacheInvalidationAcrossRefreshScopes.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java:
##########
@@ -125,11 +132,68 @@ public Table getPaimonTable(NameMapping nameMapping) {
}
public Table getPaimonTable(NameMapping nameMapping, String branch, String
queryType) {
- makeSureInitialized();
Identifier identifier = tableIdentifier(nameMapping, branch,
queryType);
return loadPaimonTable(nameMapping, queryType, identifier);
}
+ public synchronized void invalidatePaimonTable(NameMapping nameMapping)
throws Exception {
+ // Property changes reset and close the SDK catalog before retiring
Doris cache entries.
+ // Do not recreate that catalog merely to invalidate an already
retired generation.
+ if (!isInitialized()) {
+ return;
+ }
+ Identifier identifier = tableIdentifier(nameMapping, null, null);
+ withSdkCatalogCacheWriteLock(() -> executionAuthenticator.execute(()
-> {
+ catalog.invalidateTable(identifier);
+ return null;
+ }));
+ }
+
+ public synchronized void invalidatePaimonDatabase(String remoteDbName)
throws Exception {
+ if (!isInitialized()) {
+ return;
+ }
+ withSdkCatalogCacheWriteLock(() -> {
+ invalidateCachedPaimonTables(identifier ->
identifier.getDatabaseName().equals(remoteDbName));
+ return null;
+ });
+ }
+
+ public synchronized void invalidatePaimonCatalog() throws Exception {
+ if (!isInitialized()) {
+ return;
+ }
+ withSdkCatalogCacheWriteLock(() -> {
+ invalidateCachedPaimonTables(ignored -> true);
+ return null;
+ });
+ }
+
+ private void invalidateCachedPaimonTables(Predicate<Identifier> predicate)
throws Exception {
+ // A property ALTER closes the old SDK catalog before Doris retires
its cache entries.
+ // The new SDK catalog must remain lazily initialized in that callback.
+ if (!isInitialized() || !(catalog instanceof CachingCatalog)) {
Review Comment:
Addressed in the current head. findCachingCatalog() walks the public
DelegateCatalog.wrapped() chain, so PrivilegedCatalog(CachingCatalog(delegate))
is reached for table, database, and catalog invalidation. Covered by the
wrapped/case-insensitive refresh test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java:
##########
@@ -140,13 +204,12 @@ public Table getPaimonTable(NameMapping nameMapping,
String branch, String query
* Doris REFRESH can repopulate its cache with the same stale Paimon table
handle.
*/
public Table reloadPaimonTable(NameMapping nameMapping) {
- makeSureInitialized();
Identifier identifier = tableIdentifier(nameMapping, null, null);
try {
- return executionAuthenticator.execute(() -> {
+ return withInitializedSdkCatalogCacheWriteLock(() ->
executionAuthenticator.execute(() -> {
Review Comment:
Addressed in the current head. Reads take the shared read lock plus a
per-identifier striped lock; the catalog-wide write lock is reserved for
invalidation and close, and the same-key reload race is serialized per
identifier. Covered by testReloadsForDifferentTablesDoNotShareCatalogWriteLock.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonMetadataOps.java:
##########
@@ -193,7 +193,13 @@ private void performDropDb(String dbName, boolean
ifExists, boolean force) throw
@Override
public void afterDropDb(String dbName) {
+ Optional<ExternalDatabase<? extends ExternalTable>> db =
dorisCatalog.getDbForReplay(dbName);
dorisCatalog.unregisterDatabase(dbName);
+ if (db.isPresent()) {
+ Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(db.get());
+ } else {
+ invalidatePaimonCatalogForUnresolvedReplay();
Review Comment:
Fixed in 8a0670156235d7e988d681856ea0a28134431cb3.
ExternalMetadataOps.dropDb/dropDbImpl now report whether the remote drop
actually happened. A no-op DROP DATABASE IF EXISTS skips both afterDropDb and
the edit-log write, so neither the leader nor a replayed follower flushes the
Paimon SDK cache. Added
testDropDatabaseIfExistsNoOpDoesNotFlushSdkCacheOrJournal; the full affected
set passes (119 tests): PaimonExternalMetaCacheTest, PaimonMetadataOpsTest,
ExternalMetaCacheRouteResolverTest, RefreshCatalogTest,
ExternalCatalogDeadlockTest, DropDbTest.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java:
##########
@@ -125,11 +137,73 @@ public Table getPaimonTable(NameMapping nameMapping) {
}
public Table getPaimonTable(NameMapping nameMapping, String branch, String
queryType) {
- makeSureInitialized();
Identifier identifier = tableIdentifier(nameMapping, branch,
queryType);
return loadPaimonTable(nameMapping, queryType, identifier);
}
+ public synchronized void invalidatePaimonTable(NameMapping nameMapping)
throws Exception {
+ // Property changes reset and close the SDK catalog before retiring
Doris cache entries.
+ // Do not recreate that catalog merely to invalidate an already
retired generation.
+ if (!isInitialized()) {
+ return;
+ }
+ Identifier identifier = tableIdentifier(nameMapping, null, null);
+ withSdkCatalogCacheWriteLock(() -> executionAuthenticator.execute(()
-> {
+ invalidatePaimonTableEntries(identifier);
+ return null;
+ }));
+ }
+
+ public synchronized void invalidatePaimonDatabase(String remoteDbName)
throws Exception {
+ if (!isInitialized()) {
+ return;
+ }
+ withSdkCatalogCacheWriteLock(() -> executionAuthenticator.execute(()
-> {
+ boolean caseSensitive = catalog.caseSensitive();
+ invalidateCachedPaimonTables(identifier -> identifierPartEquals(
+ identifier.getDatabaseName(), remoteDbName,
caseSensitive));
+ return null;
+ }));
+ }
+
+ public synchronized void invalidatePaimonCatalog() throws Exception {
+ if (!isInitialized()) {
+ return;
+ }
+ withSdkCatalogCacheWriteLock(() -> executionAuthenticator.execute(()
-> {
+ invalidateCachedPaimonTables(ignored -> true);
+ return null;
+ }));
+ }
+
+ private void invalidateCachedPaimonTables(Predicate<Identifier> predicate)
throws Exception {
+ // A property ALTER closes the old SDK catalog before Doris retires
its cache entries.
+ // The new SDK catalog must remain lazily initialized in that callback.
+ if (!isInitialized()) {
+ return;
+ }
+ CachingCatalog cachingCatalog = findCachingCatalog();
+ if (cachingCatalog == null) {
+ return;
+ }
+ List<Identifier> cachedIdentifiers = new
ArrayList<>(cachingCatalog.tableCache().asMap().keySet());
+ List<Identifier> matchedIdentifiers = new ArrayList<>();
+ for (Identifier identifier : cachedIdentifiers) {
+ if (predicate.test(identifier)) {
+ matchedIdentifiers.add(identifier);
+ }
+ }
+ if (matchedIdentifiers.isEmpty()) {
+ return;
+ }
+ // Remove all matching branch/system table handles in one pass.
Calling Paimon's
+ // invalidateTable before this bulk removal would rescan every
unrelated database key.
+ cachingCatalog.tableCache().invalidateAll(matchedIdentifiers);
+ // Also clear Paimon's partition cache. The table cache is already
empty, so these calls
+ // do not repeatedly walk unrelated table keys.
+ matchedIdentifiers.forEach(cachingCatalog::invalidateTable);
Review Comment:
Fixed in 8a0670156235d7e988d681856ea0a28134431cb3. Database-scope
invalidation now removes the matched table handles with one
tableCache().invalidateAll(matched) pass and clears the matched partition-cache
keys in a single batch instead of calling CachingCatalog.invalidateTable once
per matched key. Paimon 1.4.2 exposes no batch partition accessor, so the
protected partitionCache field is resolved once, with a documented per-key
fallback if a future build renames it. Added
testDatabaseInvalidationClearsPartitionCacheInOneBatch (16 matched + 16
unrelated; asserts CachingCatalog.invalidateTable is never called and unrelated
partition keys are retained).
--
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]
