github-actions[bot] commented on code in PR #68238:
URL: https://github.com/apache/doris/pull/68238#discussion_r4078757890
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonMetadataOps.java:
##########
@@ -358,11 +382,19 @@ private void performDropTable(String dBName, String
tableName, boolean ifExists)
@Override
public void afterDropTable(String dbName, String tblName) {
Optional<ExternalDatabase<?>> db = dorisCatalog.getDbForReplay(dbName);
- db.ifPresent(externalDatabase ->
externalDatabase.unregisterTable(tblName));
+ boolean invalidated = db.isPresent() &&
db.get().unregisterTableForReplay(tblName);
+ if (!invalidated) {
+ invalidatePaimonCatalogForUnresolvedReplay();
Review Comment:
[P1] Retire a retained database object on this unresolved table-drop path
too. `DROP TABLE` journals the caller's raw database spelling, so with
`lower_case_database_names=2` a follower can lose the lowercase mapping while
the canonical `ExternalDatabase` remains in the independent object cache.
Replaying an alternate-case log then misses `db` here and only flushes Paimon's
engine group; a same-name recreation can restore the mapping and reuse the old
database's complete nested table-name/object cache. This is distinct from the
resolved SDK-only replay thread and from the DROP DATABASE fix, because
`afterDropTable` never calls
`retireAllDatabaseObjectsWithoutEngineInvalidation`. Conservatively retire the
retained legacy database object(s) before this fallback and cover
alternate-case replay across a lost mode-2 mapping and same-name recreation.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/paimon/PaimonTableLoader.java:
##########
@@ -32,6 +33,34 @@
*/
public final class PaimonTableLoader {
+ public void invalidate(NameMapping nameMapping) {
+ try {
+ catalog(nameMapping).invalidatePaimonTable(nameMapping);
+ } catch (Exception e) {
+ throw new CacheException("failed to invalidate paimon table
%s.%s.%s: %s",
Review Comment:
[P1] Keep this post-drop cache failure from suppressing the committed
table-drop log. `dropTableImpl` has already completed `catalog.dropTable(...)`
before `afterDropTable` reaches this new invalidator. If
`invalidatePaimonTable` throws, this wrapper raises `CacheException`;
`ExternalCatalog.dropTable` then exits before writing `DropInfo`. The leader
reports a failed DDL after the remote table was deleted, while followers never
replay the drop and can retain stale metadata. Make the post-drop SDK cleanup
best-effort or otherwise guarantee the successful remote mutation is journaled
before propagating its failure, and cover an injected invalidation failure.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/operations/ExternalMetadataOps.java:
##########
@@ -78,15 +78,34 @@ default void afterCreateDb() {
* @param force
* @throws DdlException
*/
- default void dropDb(String dbName, boolean ifExists, boolean force) throws
DdlException {
- dropDbImpl(dbName, ifExists, force);
+ default boolean dropDb(String dbName, boolean ifExists, boolean force)
throws DdlException {
+ if (!dropDbImpl(dbName, ifExists, force)) {
+ // No remote mutation happened, so do not run the post-drop hook
or journal the
+ // operation. A retained local incarnation may still need cleanup
(for example a lost
+ // case-insensitive name mapping), so give the implementation a
separate hook.
+ afterDropDbNoOp(dbName);
+ return false;
+ }
afterDropDb(dbName);
+ return true;
}
- void dropDbImpl(String dbName, boolean ifExists, boolean force) throws
DdlException;
+ /**
+ * @return whether the remote database was dropped. Returns {@code false}
when the call was a
+ * no-op (for example {@code IF EXISTS} on a database that does
not exist).
+ */
+ boolean dropDbImpl(String dbName, boolean ifExists, boolean force) throws
DdlException;
void afterDropDb(String dbName);
+ /**
+ * Cleanup hook for a drop that did not mutate the remote metastore (for
example
+ * {@code DROP DATABASE IF EXISTS} on a database that does not exist).
Implementations use it to
+ * retire any retained local object without the cache work of a real drop.
+ */
+ default void afterDropDbNoOp(String dbName) {
Review Comment:
[P1] Preserve local no-op cleanup for the other database-dropping
connectors. In mode 2, a names refresh can observe an out-of-band drop and
remove `lowerCaseToDatabaseName` while the canonical `ExternalDatabase` remains
in the independent object cache. Hive, Iceberg, and MaxCompute then return
`false` from `dropDbImpl` even for the canonical spelling, and this empty
default hook replaces the old `afterDropDb ->
unregisterDatabase(canonicalName)` cleanup. A same-name recreation can restore
the mapping and reuse the prior database's nested table-name/object cache.
Paimon handles this state in its override, but the other three implementations
do not. Give every connector targeted retained-object cleanup on the no-op path
and add lost-mode-2 mapping/recreation coverage.
--
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]
