github-actions[bot] commented on code in PR #69252:
URL: https://github.com/apache/doris/pull/69252#discussion_r4309976829
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -779,17 +779,8 @@ public ExternalDatabase<? extends ExternalTable>
getDbNullable(String dbName) {
if (StringUtils.isEmpty(dbName)) {
return null;
}
- try {
- makeSureInitialized();
- } catch (Exception e) {
- String diagnosticTrace = initErrorStackTrace(e);
- if (diagnosticTrace != null) {
- LOG.warn("failed to get db {} in catalog {}: {}", dbName,
name, diagnosticTrace);
- } else {
- LOG.warn("failed to get db {} in catalog {}", dbName, name, e);
- }
- return null;
- }
+ // Initialization errors do not establish that the requested database
is absent.
+ makeSureInitialized();
Review Comment:
[P2] Handle propagated lookup failures in COM_FIELD_LIST
`MysqlConnectProcessor.handleFieldList()` calls this method before entering
its exception handler. If a session has selected an external database and a
subsequent catalog property change makes reinitialization fail,
`COM_FIELD_LIST` now throws out of `dispatch()` before `finalizeCommand()`.
`ReadListener` catches that exception by killing and cleaning up the
connection, so the client loses its session without receiving the underlying
error. Previously the null result reached the handler's database-error
response. Extend that caller's catch-and-convert boundary to include database
lookup when enabling propagation here, and cover this protocol path with a
regression test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1098,42 +1089,14 @@ protected ExternalDatabase<? extends ExternalTable>
buildDbForInit(String remote
// When running ut, disable this check to make ut pass.
// Because in ut, the database is not created in remote system.
if (checkExists && (!FeConstants.runningUnitTest || this instanceof
TestExternalCatalog)) {
- try {
- final String lookupLocalDbName = localDbName;
- // Reuse the shared names lookup helper so existence checks
follow the same miss-refresh policy.
- Boolean exists =
resolveDatabaseNameFromSnapshot(lookupLocalDbName, false,
- namesValue ->
namesValue.containsLocalName(lookupLocalDbName) ? Boolean.TRUE : null);
- if (!Boolean.TRUE.equals(exists)) {
- LOG.warn("Database {} does not exist in the remote system.
Skipping initialization.",
- localDbName);
- return null;
- }
- } catch (RuntimeException e) {
- // Handle "Found conflicting" exception explicitly
- if (e.getMessage().contains(FOUND_CONFLICTING)) {
- LOG.error(e.getMessage());
- throw e; // Rethrow to let the caller handle this critical
issue
- } else {
- // Any errors other than name conflicts, we default to not
finding the database
- String diagnosticTrace = initErrorStackTrace(e);
- if (diagnosticTrace != null) {
- LOG.warn("Failed to check db {} exist in remote
system, ignore it: {}",
- localDbName, diagnosticTrace);
- } else {
- LOG.warn("Failed to check db {} exist in remote
system, ignore it.", localDbName, e);
- }
- return null;
- }
- } catch (Exception e) {
- // If connection failed, it will throw exception.
- // ignore it and treat it as not exist.
- String diagnosticTrace = initErrorStackTrace(e);
- if (diagnosticTrace != null) {
- LOG.warn("Failed to check db {} exist in remote system,
ignore it: {}",
- localDbName, diagnosticTrace);
- } else {
- LOG.warn("Failed to check db {} exist in remote system,
ignore it.", localDbName, e);
- }
+ // Authentication and connection failures must propagate; only a
successful lookup can prove absence.
+ final String lookupLocalDbName = localDbName;
+ // Reuse the shared names lookup helper so existence checks follow
the same miss-refresh policy.
+ Boolean exists =
resolveDatabaseNameFromSnapshot(lookupLocalDbName, false,
Review Comment:
[P2] Keep failed default-database restoration from blocking forwarded DDL
For a session on a follower that previously selected `ext.db`,
`FEOpExecutor` forwards that default catalog/database with every redirected
command. On the master, `ConnectProcessor.proxyExecute()` calls
`getDbNullable(defaultDatabase)` before `executor.queryRetry()`. If the
master's mode-0 database cache is cold and listing now fails (for example after
a metastore outage or credential change), this newly propagated exception
aborts even `REFRESH CATALOG ext` or `ALTER CATALOG ext SET PROPERTIES (...)`
before the command can repair/reset the catalog. Previously this existence
check returned null and optional context restoration was skipped. Update that
restoration path to tolerate lookup failure for commands that do not require
the default database, while preserving errors for actual database access.
--
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]
