Fix time-ordered scan queries on realtime segments#7546
Merged
jon-wei merged 7 commits intoapache:masterfrom Apr 26, 2019
Merged
Fix time-ordered scan queries on realtime segments#7546jon-wei merged 7 commits intoapache:masterfrom
jon-wei merged 7 commits intoapache:masterfrom
Conversation
Contributor
|
Tagged 0.15.0 since it fixes bugs that were introduced in patches that are new in 0.15.0. |
jon-wei
reviewed
Apr 26, 2019
|
|
||
| // If timestamp is < Integer.MAX_VALUE, it'll be an Integer object which can't be cast to a Long. This method | ||
| // checks the type of the timestamp object and converts to a long value | ||
| private long convertTimestampObjectToLong(Object timestampObj) |
Contributor
There was a problem hiding this comment.
This could use DimensionHandlerUtils.convertObjectToLong() instead
jon-wei
reviewed
Apr 26, 2019
| final List<ScanResultValue> results4 = | ||
| QueryPlus.wrap(query4).run(appenderator, new HashMap<>()).toList(); | ||
| Assert.assertEquals(2, results4.size()); // 1 per segment | ||
| // Should return the 2 rows with `met` values of 8 and 64 (based on the segment spec provided) |
Contributor
There was a problem hiding this comment.
Can you add checks for the values within the results?
jon-wei
approved these changes
Apr 26, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch fixes the following 2 bugs introduced in #7133:
ClassCastException is thrown when a timestamp less than
Integer.MAX_VALUEis being scan queried with a ASCENDING or DESCENDING time-ordering. The original timestamp comparator was written to assume that all timestamps are longs. However, if a timestamp less than the maximum integer value is used, the broker will parse the value as an Integer. Since Integers can't be cast to Longs, an exception is thrown. This patch introduces a type check for timestamps that properly parses it if it's either an Integer or a Long.UnsupportedOperationException is thrown when time-ordered scan querying a realtime segment. In the case of published segments, there's a 1:1 mapping between query runners and segment descriptors. The existing implementation relied on this mapping to calculate which query runners corresponded to each interval. However, this mapping doesn't exist when querying a realtime segment since a query runner is generated for each hydrant but only one segment descriptor is generated. This patch introduces a
SinkQueryRunnersclass that implementsIterable<QueryRunner>and holds mappings between intervals and query runners.This patch was validated on an in-house cluster.
The following types of queries have been tested for regressions: