-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Enhancement](external catalog) Added status reset when jdbc name mapping is abnormal #33971
Conversation
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
run buildall |
TPC-H: Total hot run time: 38537 ms
|
TPC-DS: Total hot run time: 185126 ms
|
ClickBench: Total hot run time: 30 s
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to log level to warn
loadColumnNamesIfNeeded(localDbName, localTableName); | ||
} | ||
return localColumnToRemoteColumn.get(remoteDbName).get(remoteTableName); | ||
if (columnMap.isEmpty()) { | ||
LOG.error("No remote column found for localTableName: {}. Please refresh this catalog.", localTableName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOG.error("No remote column found for localTableName: {}. Please refresh this catalog.", localTableName); | |
LOG.warn("No remote column found for localTableName: {}. Please refresh this catalog.", localTableName); |
fe/fe-core/src/main/java/org/apache/doris/datasource/mapping/IdentifierMapping.java
Show resolved
Hide resolved
loadDatabaseNames(); | ||
} catch (Exception e) { | ||
dbNamesLoaded.set(false); // Reset on failure | ||
LOG.error("Error loading database names", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOG.error("Error loading database names", e); | |
LOG.warn("Error loading database names", e); |
206cf00
to
56850a6
Compare
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
run cloud_p1 |
run buildall |
TPC-H: Total hot run time: 41040 ms
|
TPC-DS: Total hot run time: 186192 ms
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Proposed changes
Issue Number: close #xxx
This PR introduces several important improvements aimed at enhancing the robustness, maintainability, and error handling efficiency of database connectivity and data retrieval processes. Here are the main optimization points:
Code Abstraction and Reuse:
getRequiredMapping
method to centralize the common logic for retrieving database, table, and column names. This change reduces repetitive code across multiple functions, making the overall code structure more compact and consistent.Unified Error Handling:
getRequiredMapping
method, missing mappings are handled uniformly. If a mapping is not found, the corresponding loading function is automatically triggered, and an exception is thrown if data remains missing. This approach simplifies error management and enhances code readability and maintainability.Enhanced Exception Safety:
Improved Robustness:
ConcurrentHashMap
andAtomicBoolean
to ensure data consistency and thread safety in a multi-threaded environment. Optimized data checks and exception handling logic reduce potential issues caused by concurrent modifications.Improved Logging:
Further comments
If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...