Gabriel39 commented on code in PR #69252:
URL: https://github.com/apache/doris/pull/69252#discussion_r4310093748
##########
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 d632b8c8ae6. The field-list exception boundary now includes
database/table lookup and lock acquisition; the table lock is released in
finally and EOF is set only after success. The new processOnce tests cover both
catalog initialization and database-discovery failures, verify an ERR packet
containing the original error, and verify that a subsequent PING succeeds on
the same session.
##########
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 d632b8c8ae6. Only a failed optional database lookup is deferred;
the selected database name is retained with an unresolved ID so actual access
resolves it again instead of reporting no selected database. Successful
restoration still calls Env.changeDb outside the catch. Tests execute forwarded
REFRESH CATALOG and ALTER CATALOG, verify that actual database access still
reports the metadata failure, and verify healthy restoration and denial of
ALTER for an unprivileged user without changing catalog properties.
--
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]
