ankitsultana commented on code in PR #17278:
URL: https://github.com/apache/pinot/pull/17278#discussion_r2628315253
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/timeseries/TimeSeriesExchangeReceiveOperator.java:
##########
@@ -193,11 +213,25 @@ private TimeSeriesBlock getNextBlockNoAggregation()
Map<Long, List<TimeSeries>> timeSeriesMap = new HashMap<>();
TimeBuckets timeBuckets = null;
Map<String, String> aggregatedStats = new HashMap<>();
+ QueryException timeoutException = null;
for (int index = 0; index < _numServersQueried; index++) {
long remainingTimeMs = _deadlineMs - System.currentTimeMillis();
- Preconditions.checkState(remainingTimeMs > 0, "Timed out before polling
exchange receive");
+ if (remainingTimeMs <= 0) {
+ timeoutException = new
QueryException(QueryErrorCode.SERVER_NOT_RESPONDING,
Review Comment:
same as above
##########
pinot-timeseries/pinot-timeseries-spi/src/main/java/org/apache/pinot/tsdb/spi/series/TimeSeriesBlock.java:
##########
@@ -41,7 +43,11 @@ public class TimeSeriesBlock {
/**
* Holds optional metadata about the block (e.g., statistics).
*/
- private Map<String, String> _metadata;
+ private final Map<String, String> _metadata;
+ /**
+ * Holds exceptions encountered during processing of the block.
+ */
+ private final List<QueryException> _exceptions;
Review Comment:
Let's leave a `TODO(timeseries)` for this here. Also callout that exceptions
are not serialized and propagated from servers to brokers at the moment
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/timeseries/TimeSeriesExchangeReceiveOperator.java:
##########
@@ -116,13 +118,27 @@ private TimeSeriesBlock getNextBlockWithAggregation()
TimeBuckets timeBuckets = null;
Map<Long, BaseTimeSeriesBuilder> seriesBuilderMap = new HashMap<>();
Map<String, String> aggregatedStats = new HashMap<>();
+ QueryException timeoutException = null;
+
for (int index = 0; index < _numServersQueried; index++) {
// Step-1: Poll, and ensure we received a TimeSeriesBlock.
long remainingTimeMs = _deadlineMs - System.currentTimeMillis();
- Preconditions.checkState(remainingTimeMs > 0,
- "Timed out before polling all servers. Successfully Polled: %s of
%s", index, _numServersQueried);
+ if (remainingTimeMs <= 0) {
Review Comment:
I think it might be neater to handle negative `remainingTimeMs` in the
`poll` method and simply return `null` there
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/timeseries/TimeSeriesExchangeReceiveOperator.java:
##########
@@ -116,13 +118,27 @@ private TimeSeriesBlock getNextBlockWithAggregation()
TimeBuckets timeBuckets = null;
Map<Long, BaseTimeSeriesBuilder> seriesBuilderMap = new HashMap<>();
Map<String, String> aggregatedStats = new HashMap<>();
+ QueryException timeoutException = null;
+
for (int index = 0; index < _numServersQueried; index++) {
// Step-1: Poll, and ensure we received a TimeSeriesBlock.
long remainingTimeMs = _deadlineMs - System.currentTimeMillis();
- Preconditions.checkState(remainingTimeMs > 0,
- "Timed out before polling all servers. Successfully Polled: %s of
%s", index, _numServersQueried);
+ if (remainingTimeMs <= 0) {
+ timeoutException = new
QueryException(QueryErrorCode.SERVER_NOT_RESPONDING,
+ "Timed out before polling all servers. Successfully Polled: %s of %s"
+ + index
+ + " of "
+ + _numServersQueried);
+ break;
+ }
Object result = poll(remainingTimeMs);
- Preconditions.checkNotNull(result, "Timed out waiting for response.
Waited: %s ms", remainingTimeMs);
+ if (result == null) {
+ timeoutException = new
QueryException(QueryErrorCode.SERVER_NOT_RESPONDING,
Review Comment:
in the future would we be able to also list the host:port that didn't
respond? That's quite useful
--
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]
