Copilot commented on code in PR #60406:
URL: https://github.com/apache/doris/pull/60406#discussion_r2750093563
##########
fe/fe-core/src/main/java/org/apache/doris/system/HeartbeatMgr.java:
##########
@@ -374,7 +375,13 @@ private HeartbeatResponse pingOnce() {
? "Unknown error" :
result.getStatus().getErrorMsgs().get(0));
}
} catch (Exception e) {
- LOG.warn("backend heartbeat got exception", e);
+ Throwable rootCause = Util.getRootCause(e);
+ if (rootCause instanceof java.net.ConnectException) {
Review Comment:
The current implementation only suppresses `ConnectException`, but there are
other common network-related exceptions that occur during backend
unavailability and would also benefit from suppressed stack traces. Consider
also checking for:
- `java.net.SocketTimeoutException` (connection timeout)
- `java.net.NoRouteToHostException` (network unreachable)
- `java.net.SocketException` (general socket errors)
These exceptions are equally expected when backends are down and provide no
additional diagnostic value with full stack traces.
```suggestion
if (rootCause instanceof java.net.ConnectException
|| rootCause instanceof
java.net.SocketTimeoutException
|| rootCause instanceof
java.net.NoRouteToHostException
|| rootCause instanceof java.net.SocketException) {
```
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -795,8 +795,14 @@ public void executeDynamicPartition(Collection<Pair<Long,
Long>> dynamicPartitio
clearCreatePartitionFailedMsg(olapTable.getId());
} catch (Exception e) {
recordCreatePartitionFailedMsg(db.getFullName(),
tableName, e.getMessage(), olapTable.getId());
- LOG.warn("db [{}-{}], table [{}-{}]'s dynamic
partition has error",
- db.getId(), db.getName(), olapTable.getId(),
olapTable.getName(), e);
+ String errMsg = e.getMessage();
+ if (errMsg != null && errMsg.contains("available
backend")) {
Review Comment:
Using string matching with `errMsg.contains("available backend")` is fragile
and error-prone. This approach has several issues:
1. It's case-sensitive and will fail if the error message case changes
2. It could match unintended error messages containing "available backend"
in different contexts
3. The actual error messages vary across the codebase (e.g., "No available
backend", "no available backend", "replication num should be less than the
number of available backends")
Consider a more robust approach:
- Check for specific exception types (e.g., `DdlException` with specific
causes)
- Use a more specific substring that uniquely identifies backend
availability errors
- Or define a custom exception type for backend availability issues
For reference, some actual error messages in the codebase include:
- "No available backends" (FederationBackendPolicy.java:187)
- "No available backend" (LocalTableValuedFunction.java:103)
- "replication num should be less than the number of available backends"
(SystemInfoService.java:527)
```suggestion
if (errMsg != null &&
(StringUtils.containsIgnoreCase(errMsg, "no available backend")
|| StringUtils.containsIgnoreCase(errMsg,
"no available backends")
|| StringUtils.containsIgnoreCase(errMsg,
"number of available backends"))) {
```
--
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]
