gortiz commented on code in PR #15919:
URL: https://github.com/apache/pinot/pull/15919#discussion_r2113602711
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/blocks/ErrorMseBlock.java:
##########
@@ -24,43 +24,75 @@
import java.util.Collections;
import java.util.EnumMap;
import java.util.Map;
-import javax.validation.constraints.NotNull;
-import org.apache.pinot.common.datablock.DataBlockUtils;
+import javax.annotation.Nullable;
import org.apache.pinot.common.response.ProcessingException;
+import org.apache.pinot.common.utils.ExceptionUtils;
+import org.apache.pinot.query.MseWorkerThreadContext;
import org.apache.pinot.spi.exception.QueryErrorCode;
import org.apache.pinot.spi.exception.QueryException;
+import org.apache.pinot.spi.query.QueryThreadContext;
import org.apache.pinot.spi.utils.JsonUtils;
/// A block that represents a failed execution.
///
public class ErrorMseBlock implements MseBlock.Eos {
+ private final int _stage;
+ private final int _worker;
+ private final String _serverId;
private final EnumMap<QueryErrorCode, String> _errorMessages;
+ /// Kept for backward compatibility.
+ ///
+ /// @deprecated Use [#fromMap(Map)] instead
+ @Deprecated
public ErrorMseBlock(Map<QueryErrorCode, String> errorMessages) {
+ this(-1, -1, null, errorMessages);
+ }
+
+ public ErrorMseBlock(int stage, int worker, String serverId,
Map<QueryErrorCode, String> errorMessages) {
+ _stage = stage;
+ _worker = worker;
+ _serverId = serverId;
Preconditions.checkArgument(!errorMessages.isEmpty(), "Error messages
cannot be empty");
_errorMessages = new EnumMap<>(errorMessages);
}
+ public static ErrorMseBlock fromMap(Map<QueryErrorCode, String>
errorMessages) {
+ int stage;
+ int worker;
+ String server;
+ if (MseWorkerThreadContext.isInitialized()) {
+ stage = MseWorkerThreadContext.getStageId();
+ worker = MseWorkerThreadContext.getWorkerId();
+ server = QueryThreadContext.getServiceId();
+ } else {
+ stage = -1; // Default value when not initialized
+ worker = -1; // Default value when not initialized
+ server = null; // Default value when not initialized
+ }
+ return new ErrorMseBlock(stage, worker, server, errorMessages);
+ }
+
public static ErrorMseBlock fromException(Exception e) {
QueryErrorCode errorCode;
+ boolean extractTrace;
if (e instanceof QueryException) {
errorCode = ((QueryException) e).getErrorCode();
+ extractTrace = false;
} else if (e instanceof ProcessingException) {
errorCode = QueryErrorCode.fromErrorCode(((ProcessingException)
e).getErrorCode());
+ extractTrace = true;
} else {
errorCode = QueryErrorCode.UNKNOWN;
+ extractTrace = true;
}
- String errorMessage = shouldIncludeStackTrace(errorCode) ?
DataBlockUtils.extractErrorMsg(e) : e.getMessage();
- return new ErrorMseBlock(Collections.singletonMap(errorCode,
errorMessage));
+ String errorMessage = extractTrace ?
ExceptionUtils.consolidateExceptionMessages(e) : e.getMessage();
+ return fromMap(Collections.singletonMap(errorCode, errorMessage));
Review Comment:
New semantic:
1. If the exception was a QueryException, we assume the message is the one
we want to send to the user. No extra processing is done, which means if there
is a cause, we don't include its message.
2. On any other case, we iterate over the causes and print the message (but
not the stack trace) of the whole exception chain.
Previously, we only did that for exceptions whose error code was UNKNOWN.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
