kfaraz commented on issue #11165:
URL: https://github.com/apache/druid/issues/11165#issuecomment-832919549
> How would the proposed error-code-based system integrate with the current
ingestion system? Looking at the code snippets, it seems like we have to modify
all codes where throw exceptions to convert them to DruidTypedException? If
this is what you are proposing, it seems pretty error-prone because it's easy
to forget catching and converting exceptions especially when they are unchecked
exceptions. Also, please add a code snippet of TaskStatus after this change. It
will help to understand the proposal.
In case of ingestion, the catch-all error handling is already being done in
`TaskQueue`. We would need to modify it to handle the new type of exception as
below. This would also require converting the exceptions that we throw from the
known failure points to the new exception i.e. the `DruidTypedException`
discussed above.
```java
public class TaskQueue {
// ...
private ListenableFuture<TaskStatus> attachCallbacks(final Task task) {
// ...
Futures.addCallback(new FutureCallback<TaskStatus> {
@Override
public void onSuccess(final TaskStatus successStatus) {
// persist the successStatus
}
@Override
public void onFailure(final Throwable t) {
TaskStatus failureStatus;
if (t instanceof DruidTypedException) {
failureStatus =
TaskStatus.failure(task.getId(), ((DruidTypedException)
t).getErrorTypeParams());
} else {
// build a generic error message here
failureStatus =
TaskStatus.failure(task.getId(), ...);
}
// persist the failureStatus
}
});
}
// ...
}
```
Alongwith this, any other place where we return a `TaskStatus.failure()`
would get modified as below:
```java
TaskStatus status = TaskStatus.failure(
taskId,
ErrorTypeParams.of(moduleName, errorCode, messageArgs)
);
```
The primary change to the `TaskStatus` class is as under (also added to the
proposal above):
```java
public class TaskStatus {
// Existing field
private final @Nullable String errorMsg;
// New field. Contains moduleName, errorCode and messageArgs.
// A TaskStatus with this field as null would fall back
// to the existing field errorMsg
private final @Nullable ErrorTypeParams errorTypeParams;
//...
}
```
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
