gortiz commented on code in PR #15037:
URL: https://github.com/apache/pinot/pull/15037#discussion_r1956074972


##########
pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerQueryErrorMessage.java:
##########
@@ -18,24 +18,39 @@
  */
 package org.apache.pinot.common.response.broker;
 
+import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.pinot.spi.exception.QueryErrorCode;
+import org.apache.pinot.spi.exception.QueryErrorMessage;
 
 
 /**
  * This class represents an exception using a message and an error code.
+ *
+ * This is only used to serialize the error message and error code when a 
broker sends an error message to the client.
+ * In other cases use {@link QueryErrorMessage} instead.
  */
-public class QueryProcessingException {
+public class BrokerQueryErrorMessage {

Review Comment:
   Good question. The answer could be added to the Javadoc as well.
   
   In the code currently in master, we use ProcessingException in two cases:
   1. We throw that exception when we detect some errors at runtime.
   2. We use them to store error messages in blocks we use to transfer 
information between operators and nodes.
   
   The first case, throw exceptions, is addressed by `QueryException`. The 
second case is handled by `QueryErrorMessage`, which is entirely internal. In 
contrast, `BrokerQueryErrorMessage` is not. It is essentially a new name for 
QueryProcessingException (which was not an actual Java exception) and is 
utilized as part of the `BrokerResponseNative`. This means that 
`BrokerQueryErrorMessage` belongs to the presentation layer, and its content, 
form, and semantics depend on what we communicate to the user. We must also be 
very meticulous about backward compatibility.
   
   In `QueryErrorMessage`, I've added extra content such as the log message, 
and I plan to include more contextual information, like on which stage the 
error originated (which I found useful for the MSE send operator to know if the 
error was raised in the current or a child stage). 



-- 
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

Reply via email to