-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Time Ordering On Scans #7133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
jon-wei
merged 105 commits into
apache:master
from
justinborromeo:6088-Time-Ordering-On-Scans-N-Way-Merge
Mar 28, 2019
Merged
Time Ordering On Scans #7133
Changes from all commits
Commits
Show all changes
105 commits
Select commit
Hold shift + click to select a range
10e57d5
Moved Scan Builder to Druids class and started on Scan Benchmark setup
justinborromeo dba6e49
Merge branch 'master' into 6088-Create-Scan-Benchmark
justinborromeo dd4ec1a
Need to form queries
justinborromeo 26930f8
It runs.
justinborromeo 7a6080f
Stuff for time-ordered scan query
justinborromeo 79e8319
Move ScanResultValue timestamp comparator to a separate class for tes…
justinborromeo 7b58471
Licensing stuff
justinborromeo 989bd2d
Merge branch '6088-Create-Scan-Benchmark' into 6088-Time-Ordering-On-…
justinborromeo ad731a3
Change benchmark
justinborromeo e66339c
Remove todos
justinborromeo 12e51a2
Added TimestampComparator tests
justinborromeo 432acaf
Change number of benchmark iterations
justinborromeo 01b25ed
Added time ordering to the scan benchmark
justinborromeo 9e6e716
Changed benchmark params
justinborromeo 20c3664
More param changes
justinborromeo 796083f
Benchmark param change
justinborromeo 737a833
Made Jon's changes and removed TODOs
justinborromeo b7d3a49
Merge branch 'master' into 6088-Time-Ordering-On-Scans-V2
justinborromeo 86c5eee
Broke some long lines into two lines
justinborromeo 7deb06f
Merge branch '6088-Create-Scan-Benchmark' into 6088-Time-Ordering-On-…
justinborromeo d1a1793
nit
justinborromeo b6d4df3
Decrease segment size for less memory usage
justinborromeo 8b7d5f5
Wrote tests for heapsort scan result values and fixed bug where iterator
justinborromeo 4f51024
Wrote more tests for scan result value sort
justinborromeo 60b7684
Committing a param change to kick teamcity
justinborromeo 5edbe2a
Merge github.com:apache/incubator-druid into 6088-Create-Scan-Benchmark
justinborromeo 148939e
Merge branch '6088-Create-Scan-Benchmark' into 6088-Time-Ordering-On-…
justinborromeo dfe4aa9
Fixed codestyle and forbidden API errors
justinborromeo 10b5e0c
.
justinborromeo 8212a21
Improved conciseness
justinborromeo 305876a
nit
justinborromeo e8a4b49
Merge branch 'master' into 6088-Time-Ordering-On-Scans-V2
justinborromeo 7e872a8
Created an error message for when someone tries to time order a result
justinborromeo 85e72a6
Set to spaces over tabs
justinborromeo b2c8c77
Fixing tests WIP
justinborromeo b432bea
Fixed failing calcite tests
justinborromeo ab00ead
Kicking travis with change to benchmark param
justinborromeo d3b335a
added all query types to scan benchmark
justinborromeo 2e3577c
Fixed benchmark queries
justinborromeo 134041c
Renamed sort function
justinborromeo 93e1636
Added javadoc on ScanResultValueTimestampComparator
justinborromeo 5f92dd7
Unused import
justinborromeo f0eddee
Added more javadoc
justinborromeo ecb0f48
improved doc
justinborromeo 4e69276
Removed unused import to satisfy PMD check
justinborromeo 35150fe
Small changes
justinborromeo 7baeade
Changes based on Gian's comments
justinborromeo cd489a0
Fixed failing test due to null resultFormat
justinborromeo c9142e7
Merge branch 'master' into 6088-Time-Ordering-On-Scans-V2
justinborromeo fba6b02
Added config and get # of segments
justinborromeo b13ff62
Set up time ordering strategy decision tree
justinborromeo f83e996
Refactor and pQueue works
justinborromeo 1813a54
Cleanup
justinborromeo f57ff25
Ordering is correct on n-way merge -> still need to batch events into
justinborromeo e1fc295
WIP
justinborromeo 023538d
Sequence stuff is so dirty :(
justinborromeo 3b923da
Fixed bug introduced by replacing deque with list
justinborromeo 06a5218
Wrote docs
justinborromeo 763c43d
Multi-historical setup works
justinborromeo 69b24bd
Merge branch 'master' into 6088-Time-Ordering-On-Scans-N-Way-Merge
justinborromeo 451e2b4
WIP
justinborromeo 18cce9a
Change so batching only occurs on broker for time-ordered scans
justinborromeo 5bd0e1a
Merge branch 'master' into 6088-Time-Ordering-On-Scans-N-Way-Merge
justinborromeo de83b11
Fixed mistakes in merge
justinborromeo 806166f
Fixed failing tests
justinborromeo 5ff59f5
Reset config
justinborromeo 47c970b
Wrote tests and added Javadoc
justinborromeo 83ec3fe
Nit-change on javadoc
justinborromeo 2d1978d
Merge branch 'master' into 6088-Time-Ordering-On-Scans-N-Way-Merge
justinborromeo 35c96d3
Checkstyle fix
justinborromeo 6dc53b3
Improved test and appeased TeamCity
justinborromeo fb966de
Sorry, checkstyle
justinborromeo 73f4038
Applied Jon's recommended changes
justinborromeo cce917a
Checkstyle fix
justinborromeo 45e95bb
Optimization
justinborromeo 57b5682
Fixed tests
justinborromeo a032c46
Updated error message
justinborromeo 7e49d47
Added error message for UOE
justinborromeo 7bfa77d
Merge branch 'Update-Query-Interrupted-Exception' into 6088-Time-Orde…
justinborromeo 2528a56
Renaming
justinborromeo 4823dab
Finish rename
justinborromeo 42f5246
Smarter limiting for pQueue method
justinborromeo 43d490c
Optimized n-way merge strategy
justinborromeo 4947216
Rename segment limit -> segment partitions limit
justinborromeo 1b46b58
Added a bit of docs
justinborromeo 62dceda
More comments
justinborromeo a87d021
Fix checkstyle and test
justinborromeo 8b3b6b5
Nit comment
justinborromeo 86d9730
Fixed failing tests -> allow usage of all types of segment spec
justinborromeo ec47028
Fixed failing tests -> allow usage of all types of segment spec
justinborromeo 8f01d8d
Revert "Fixed failing tests -> allow usage of all types of segment spec"
justinborromeo 57033f3
Merge branch '6088-Time-Ordering-On-Scans-N-Way-Merge' of github.com:…
justinborromeo b822fc7
Revert "Merge branch '6088-Time-Ordering-On-Scans-N-Way-Merge' of git…
justinborromeo da4fc66
Check type of segment spec before using for time ordering
justinborromeo 219af47
Fix bug in numRowsScanned
justinborromeo 3569268
Fix bug messing up count of rows
justinborromeo 8a6bb11
Fix docs and flipped boolean in ScanQueryLimitRowIterator
justinborromeo 376e8bf
Refactor n-way merge
justinborromeo fb858ef
Added test for n-way merge
justinborromeo 487f31f
Refixed regression
justinborromeo 480e932
Checkstyle and doc update
justinborromeo 1df50de
Merge branch 'master' into 6088-Time-Ordering-On-Scans-N-Way-Merge
justinborromeo 231a72e
Modified sequence limit to accept longs and added test for long limits
justinborromeo 07503ea
doc fix
justinborromeo 287a367
Implemented Clint's recommendations
justinborromeo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I noticed that the code that hits the historical would maybe explode if not issued a the interval with a
MultipleSpecificSegmentSpec, but there is an example query immediately after mention that the historicals can be queried that doesn't have this segment spec, and is probably confusing. Querying historicals directly seems like an advanced use case, and should maybe be moved to it's own section near the end of this document to make it clear and not necessarily encouraging it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the time-ordering section, there's this line: "Also,
time ordering is not support for queries issued directly to historicals unless a list of segments is specified." (don't mind the typo, I'll fix that right now). Is that good enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's probably ok, i think it just flows strange for me when I read the section. It's because the things aren't really related but don't really feel separated.
For future work it might be nice to overhaul the scan query documentation, maybe drop mention of the legacy mode and the
valueVectorresult format and maybe re-arrange some things. I still feel querying a historical directly is less important than describing the scan query itself, and should be pushed to the end of the docs into an 'advanced' section that describes using the scan query as a means of parallel export into another query system or whatever, but all of this can be done in a future PR