You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "shauryachats (via GitHub)" <gi...@apache.org> on 2025/11/27 01:32:53 UTC
[PR] [timeseries] Adding support for partial results in time series engine [pinot]
shauryachats opened a new pull request, #17278:
URL: https://github.com/apache/pinot/pull/17278
(no comment)
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #17278:
URL: https://github.com/apache/pinot/pull/17278#issuecomment-3583976716
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/17278?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 0.00%. Comparing base ([`0b7ead6`](https://app.codecov.io/gh/apache/pinot/commit/0b7ead68122ad317416835b8ea14592031377d99?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`038f324`](https://app.codecov.io/gh/apache/pinot/commit/038f32478cdb1c7a69a35887b32aefe44a3cfe30?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:white_check_mark: All tests successful. No failed tests found.
> :exclamation: There is a different number of reports uploaded between BASE (0b7ead6) and HEAD (038f324). Click for more details.
>
> <details><summary>HEAD has 32 uploads less than BASE</summary>
>
>| Flag | BASE (0b7ead6) | HEAD (038f324) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|0|
>|unittests|4|0|
>|temurin|9|1|
>|java-11|4|0|
>|unittests2|2|0|
>|integration|5|1|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #17278 +/- ##
=============================================
- Coverage 63.23% 0.00% -63.24%
=============================================
Files 3133 3 -3130
Lines 186068 6 -186062
Branches 28407 0 -28407
=============================================
- Hits 117661 0 -117661
+ Misses 59343 6 -59337
+ Partials 9064 0 -9064
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-63.20%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-63.24%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/17278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/17278?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "shauryachats (via GitHub)" <gi...@apache.org>.
shauryachats commented on code in PR #17278:
URL: https://github.com/apache/pinot/pull/17278#discussion_r2621659756
##########
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:
Just to clarify what I mentioned earlier: we can look at handling exceptions versus warnings for the time-series logic in a future PR, and rely on that instead of throwing raw exceptions.
Since this PR is really about enabling partial results, it probably makes sense not to change the broader error-handling approach here. That said, this does set the groundwork for tightening up the full exception-handling flow through TimeSeriesBlock in the next PR.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana merged PR #17278:
URL: https://github.com/apache/pinot/pull/17278
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #17278:
URL: https://github.com/apache/pinot/pull/17278#discussion_r2628787148
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/timeseries/TimeSeriesExchangeReceiveOperator.java:
##########
@@ -218,6 +244,10 @@ private TimeSeriesBlock getNextBlockNoAggregation()
mergeStats(aggregatedStats, blockToMerge.getMetadata());
}
Preconditions.checkNotNull(timeBuckets, "Time buckets is null in exchange receive operator");
- return new TimeSeriesBlock(timeBuckets, timeSeriesMap, aggregatedStats);
+ TimeSeriesBlock resultBlock = new TimeSeriesBlock(timeBuckets, timeSeriesMap, aggregatedStats);
+ if (timeoutException != null) {
+ resultBlock.addToExceptions(timeoutException);
Review Comment:
@shauryachats : in a future PR let's also update exception handling in the open source timeseries plugin.
cc: @raghavyadav01 : after this PR, if you are combining time series blocks in operations such as divideSeries, you should also copy over exceptions from any of the blocks and pass them to the resulting block.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "shauryachats (via GitHub)" <gi...@apache.org>.
shauryachats commented on code in PR #17278:
URL: https://github.com/apache/pinot/pull/17278#discussion_r2628931983
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/timeseries/TimeSeriesExchangeReceiveOperator.java:
##########
@@ -116,13 +118,23 @@ 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);
Object result = poll(remainingTimeMs);
- Preconditions.checkNotNull(result, "Timed out waiting for response. Waited: %s ms", remainingTimeMs);
+ if (result == null) {
Review Comment:
Added to be safe.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #17278:
URL: https://github.com/apache/pinot/pull/17278#discussion_r2628779056
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/timeseries/TimeSeriesExchangeReceiveOperator.java:
##########
@@ -116,13 +118,23 @@ 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);
Object result = poll(remainingTimeMs);
- Preconditions.checkNotNull(result, "Timed out waiting for response. Waited: %s ms", remainingTimeMs);
+ if (result == null) {
Review Comment:
I think you need to handle negative values in `poll`?
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "shauryachats (via GitHub)" <gi...@apache.org>.
shauryachats commented on code in PR #17278:
URL: https://github.com/apache/pinot/pull/17278#discussion_r2621679167
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/timeseries/TimeSeriesExchangeReceiveOperator.java:
##########
@@ -116,13 +118,23 @@ 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,
+ String.format("Timed out before polling all servers. Successfully Polled: %s of %s", index,
Review Comment:
Good catch, addressed.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "shauryachats (via GitHub)" <gi...@apache.org>.
shauryachats commented on code in PR #17278:
URL: https://github.com/apache/pinot/pull/17278#discussion_r2628434995
##########
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:
Good catch, fixed.
##########
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:
Added a TODO.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [timeseries] Adding support for partial results in time series engine [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #17278:
URL: https://github.com/apache/pinot/pull/17278#discussion_r2621602185
##########
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:
This behavior differs from the single stage engine. SSE will collect all exceptions in a server and send them along with any partial data to the broker. The broker can then take a call about whether to return the errors as a warning or whether to fail the query immediately.
Ideally we should follow the same convention and send the errors in a serialized form to the broker and let the broker choose how it wants to handle it in the future
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org