Gabriel39 commented on code in PR #69252:
URL: https://github.com/apache/doris/pull/69252#discussion_r4321579916
##########
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:
Fixed in 3aaeccb6613d. The successful optional lookup is now reused to
publish the namespace without the two subsequent metadata lookups. The database
SHOW privilege check remains outside the metadata-failure catch, and the
statement's catalog privilege checks remain intact. Deterministic injection
resets the catalog immediately after the successful probe: both forwarded
REFRESH and ALTER reproduced the failure before this change and pass
afterwards. Controls cover actual database access failing after the same reset
and denial of unprivileged database/catalog operations. Validation: 182 related
FE tests passed with zero failures/errors/skips; FE Checkstyle passed with zero
violations.
##########
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:
Fixed in 3aaeccb6613d. Env.changeCatalog captures the complete
NameSpaceContext before changing the catalog and restores it without metadata
IO if remembered/default database selection fails. The new test reproduced the
unexpected catalog change before the fix; it now verifies the original catalog,
database name and ID are all preserved, then verifies a successful retry after
recovery. Validation: 182 related FE tests passed with zero
failures/errors/skips; FE Checkstyle passed with zero violations.
##########
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:
Fixed in 3aaeccb6613d. Both HTTP entry points now retain the URL's default
catalog/database with an unresolved ID, without eagerly contacting the external
catalog. The schema test reproduces the outage with a healthy fully qualified
internal table and now passes; actual access to the unavailable external
database still reports the original error. Both endpoints retain authentication
checks. The execution tests reproduce and verify the controller-to-submitter
boundary for REFRESH/ALTER using a mocked StatementSubmitter; they do not claim
full JDBC-worker integration coverage. Forwarded repair execution is tested
separately with real command execution. Validation: 182 related FE tests passed
with zero failures/errors/skips; FE Checkstyle passed with zero violations.
--
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]
