Jackie-Jiang commented on a change in pull request #7397:
URL: https://github.com/apache/pinot/pull/7397#discussion_r703824504
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##########
@@ -63,19 +63,27 @@ public static void setMaxLinesOfStackTrace(int
maxLinesOfStackTrace) {
public static final int COMBINE_GROUP_BY_EXCEPTION_ERROR_CODE = 600;
public static final int QUERY_VALIDATION_ERROR_CODE = 700;
public static final int UNKNOWN_ERROR_CODE = 1000;
+ public static final int SEGMENTS_NOT_ONLINE_ERROR_CODE = 1001;
Review comment:
Let's not add error code larger than 1000. Also leave some space for
future errors
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##########
@@ -63,19 +63,27 @@ public static void setMaxLinesOfStackTrace(int
maxLinesOfStackTrace) {
public static final int COMBINE_GROUP_BY_EXCEPTION_ERROR_CODE = 600;
public static final int QUERY_VALIDATION_ERROR_CODE = 700;
public static final int UNKNOWN_ERROR_CODE = 1000;
+ public static final int SEGMENTS_NOT_ONLINE_ERROR_CODE = 1001;
+ public static final int SEGMENTS_NOT_ACQUIRED_ERROR_CODE = 1002;
+ public static final int SERVERS_NO_RESPONSE_ERROR_CODE = 1003;
// NOTE: update isClientError() method appropriately when new codes are added
- public static final ProcessingException JSON_PARSING_ERROR = new
ProcessingException(JSON_PARSING_ERROR_CODE);
- public static final ProcessingException JSON_COMPILATION_ERROR = new
ProcessingException(JSON_COMPILATION_ERROR_CODE);
- public static final ProcessingException PQL_PARSING_ERROR = new
ProcessingException(PQL_PARSING_ERROR_CODE);
- public static final ProcessingException ACCESS_DENIED_ERROR = new
ProcessingException(ACCESS_DENIED_ERROR_CODE);
+ public static final ProcessingException JSON_PARSING_ERROR = new
ProcessingException(
Review comment:
Please check the code format setting. Seems you are using 80 characters
per line. Pinot Style uses 120 characters instead
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
##########
@@ -167,10 +167,7 @@ public DataTable processQuery(ServerQueryRequest
queryRequest, ExecutorService e
// TODO: Change broker to watch both IdealState and ExternalView to not
query the removed segments
Review comment:
This `TODO` is already done. You may remove this comment block
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##########
@@ -63,19 +63,27 @@ public static void setMaxLinesOfStackTrace(int
maxLinesOfStackTrace) {
public static final int COMBINE_GROUP_BY_EXCEPTION_ERROR_CODE = 600;
public static final int QUERY_VALIDATION_ERROR_CODE = 700;
public static final int UNKNOWN_ERROR_CODE = 1000;
+ public static final int SEGMENTS_NOT_ONLINE_ERROR_CODE = 1001;
+ public static final int SEGMENTS_NOT_ACQUIRED_ERROR_CODE = 1002;
+ public static final int SERVERS_NO_RESPONSE_ERROR_CODE = 1003;
Review comment:
```suggestion
public static final int SERVER_NOT_RESPONDING_ERROR_CODE = 270;
```
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
##########
@@ -251,6 +248,14 @@ public DataTable processQuery(ServerQueryRequest
queryRequest, ExecutorService e
.put(MetadataKey.MIN_CONSUMING_FRESHNESS_TIME_MS.getName(),
Long.toString(minConsumingFreshnessTimeMs));
}
+ if (numSegmentsQueried > numSegmentsAcquired) {
+ String errorMessage =
+ String.format("Some segments could not be acquired: %d",
numSegmentsQueried - numSegmentsAcquired);
Review comment:
Collect the segment names unable to be acquired and put them into the
error message
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##########
@@ -63,19 +63,27 @@ public static void setMaxLinesOfStackTrace(int
maxLinesOfStackTrace) {
public static final int COMBINE_GROUP_BY_EXCEPTION_ERROR_CODE = 600;
public static final int QUERY_VALIDATION_ERROR_CODE = 700;
public static final int UNKNOWN_ERROR_CODE = 1000;
+ public static final int SEGMENTS_NOT_ONLINE_ERROR_CODE = 1001;
+ public static final int SEGMENTS_NOT_ACQUIRED_ERROR_CODE = 1002;
Review comment:
```suggestion
public static final int SERVER_SEGMENT_MISSING_ERROR_CODE = 235;
```
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/SingleConnectionBrokerRequestHandler.java
##########
@@ -126,6 +126,9 @@ protected BrokerResponseNative processBrokerRequest(long
requestId, BrokerReques
_brokerMetrics.addMeteredTableValue(rawTableName,
BrokerMeter.BROKER_RESPONSES_WITH_PROCESSING_EXCEPTIONS, 1);
}
if (numServersQueried > numServersResponded) {
+ String errorMessage = String.format("Some servers did not respond: %d",
numServersQueried - numServersResponded);
Review comment:
Let's collect the missing server name from the `ServerRoutingInstance`
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -794,6 +795,11 @@ private BrokerResponseNative handlePQLRequest(long
requestId, String query, Json
_brokerMetrics.addMeteredTableValue(rawTableName,
BrokerMeter.BROKER_RESPONSES_WITH_NUM_GROUPS_LIMIT_REACHED, 1);
}
+ if (0 != numUnavailableSegments) {
Review comment:
Let's collect the unavailable segment names and put them into the error
message
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##########
@@ -63,19 +63,27 @@ public static void setMaxLinesOfStackTrace(int
maxLinesOfStackTrace) {
public static final int COMBINE_GROUP_BY_EXCEPTION_ERROR_CODE = 600;
public static final int QUERY_VALIDATION_ERROR_CODE = 700;
public static final int UNKNOWN_ERROR_CODE = 1000;
+ public static final int SEGMENTS_NOT_ONLINE_ERROR_CODE = 1001;
Review comment:
```suggestion
public static final int BROKER_SEGMENT_UNAVAILABLE_ERROR_CODE = 310;
```
--
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]
