github-actions[bot] commented on code in PR #64043:
URL: https://github.com/apache/doris/pull/64043#discussion_r3345628521
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/PluginDrivenExternalCatalog.java:
##########
@@ -296,6 +300,67 @@ public void gsonPostProcess() throws IOException {
}
}
+ @Override
+ public void createDb(String dbName, boolean ifNotExists, Map<String,
String> properties)
+ throws DdlException {
+ makeSureInitialized();
+ LOG.info("Creating database {} in catalog {} (ifNotExists={})",
dbName, getName(), ifNotExists);
+ HashMap<String, String> props = new HashMap<>(properties);
+ props.put("jdbc_if_not_exists", String.valueOf(ifNotExists));
+ ConnectorSession session = buildConnectorSession();
+
+ // Execute remote CREATE DATABASE
+ connector.getMetadata(session).createDatabase(session, dbName, props);
+
+ // Invalidate cache so next listDatabaseNames() re-fetches from remote
+ resetMetaCacheNames();
+
+ // Write edit log so follower FEs can replay cache invalidation
+ try {
+ Env.getCurrentEnv().getEditLog().logCreateDb(new
CreateDbInfo(getName(), dbName, null));
+ } catch (Exception e) {
+ LOG.warn("Failed to log create database {} in catalog {}.",
dbName, getName(), e);
Review Comment:
This catch makes a durable metadata failure look successful to the client.
`logCreateDb()` is the only persisted record followers/restarted FEs use to
replay the cache invalidation; if `logEdit` fails after the remote CREATE
succeeds, this FE returns success while other FEs will never apply the CREATE
replay. FE edit-log failures must not be swallowed (see the persist review
guide), and the existing `ExternalCatalog.createDb()` lets the failure
propagate through the outer catch. Please remove this catch or rethrow a
`DdlException`/runtime failure after logging so the operation does not silently
commit only remotely.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/PluginDrivenExternalCatalog.java:
##########
@@ -296,6 +300,67 @@ public void gsonPostProcess() throws IOException {
}
}
+ @Override
+ public void createDb(String dbName, boolean ifNotExists, Map<String,
String> properties)
+ throws DdlException {
+ makeSureInitialized();
+ LOG.info("Creating database {} in catalog {} (ifNotExists={})",
dbName, getName(), ifNotExists);
+ HashMap<String, String> props = new HashMap<>(properties);
+ props.put("jdbc_if_not_exists", String.valueOf(ifNotExists));
+ ConnectorSession session = buildConnectorSession();
+
+ // Execute remote CREATE DATABASE
+ connector.getMetadata(session).createDatabase(session, dbName, props);
+
+ // Invalidate cache so next listDatabaseNames() re-fetches from remote
+ resetMetaCacheNames();
+
+ // Write edit log so follower FEs can replay cache invalidation
+ try {
+ Env.getCurrentEnv().getEditLog().logCreateDb(new
CreateDbInfo(getName(), dbName, null));
+ } catch (Exception e) {
+ LOG.warn("Failed to log create database {} in catalog {}.",
dbName, getName(), e);
+ }
+ }
+
+ @Override
+ public void dropDb(String dbName, boolean ifExists, boolean force) throws
DdlException {
+ makeSureInitialized();
+ LOG.info("Dropping database {} from catalog {}", dbName, getName());
+ ConnectorSession session = buildConnectorSession();
+
+ // Execute remote DROP DATABASE
+ connector.getMetadata(session).dropDatabase(session, dbName, ifExists);
+
+ // Remove from cache
+ unregisterDatabase(dbName);
+
+ // Write edit log so follower FEs can replay cache invalidation
+ try {
+ Env.getCurrentEnv().getEditLog().logDropDb(new
DropDbInfo(getName(), dbName));
+ } catch (Exception e) {
+ LOG.warn("Failed to log drop database {} in catalog {}.", dbName,
getName(), e);
Review Comment:
Same persistence issue for DROP: if the remote DROP succeeds but
`logDropDb()` fails, the caller gets success and this FE unregisters the DB,
but followers/restarted FEs will not replay the drop and can keep stale
metadata until a manual refresh or failover-specific divergence. This should
follow the existing external catalog DDL pattern and treat edit-log failure as
operation failure, not just a warning.
--
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]
