github-actions[bot] commented on code in PR #68238:
URL: https://github.com/apache/doris/pull/68238#discussion_r4059873668
##########
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:
[P2] Do not turn the `IF EXISTS` early-return path, where no remote drop was
attempted, into a catalog-wide cache flush. `performDropDb` returns before
calling `catalog.dropDatabase`, but the default drop wrapper still calls
`afterDropDb` and `ExternalCatalog.dropDb` still journals the operation. This
new empty-object fallback therefore invalidates every unrelated cached Paimon
SDK table handle and Doris Paimon entry on the leader, and replay can repeat
that full flush on each initialized follower that likewise cannot resolve a
cached database object. Preserve the conservative catalog fallback for an
unresolved replay of a real committed drop, but propagate the leader-side
no-mutation result so it skips this post-hook/log (or otherwise avoids the full
fallback), with an unrelated warm-table regression. This trigger is distinct
from the existing catalog-refresh scan thread.
##########
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:
[P2] Avoid re-running Paimon's branch scan once per key that was already
bulk-removed. `invalidateAll(matchedIdentifiers)` removes only the D
target-database keys, not the U unrelated keys, and Paimon 1.4.2
`CachingCatalog.invalidateTable` still iterates the entire remaining
`tableCache` to find branch variants. Calling it D times here therefore does
O(D*U) useless key visits while holding the catalog-wide write lock, so a
direct database refresh can block all fenced SDK table-handle reads for a long
time. Clear the matched partition keys through a batch/non-scanning path and
add a many-target/many-unrelated regression. This is distinct from the existing
catalog-refresh thread: its per-database callback multiplication is suppressed,
while this amplification occurs inside one database invalidation.
--
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]
