Support for MongoDB time-series collections #173
Conversation
This file contains 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
1) HighCPUForHosts panics when trying to generate a query for zero hostnames 2) Option parsing code used mongo-use-native rather than mongo-use-naive for "naive" schema 3) Helper scripts didn't have a way to choose between bucketized and document-per schema
1) GroupByTimeAndPrimaryTag used 2 sorts (on b and then a) when a sort on (a,b) was needed. The double sort is probably inefficient and only correct when a stable sort is guaranteed. 2) LastPointPerHost had a predicate on the "measurement" field which did not exist in the pipeline at that stage, so the query had an empty result
|
|
|
Hello @gregorynoma ! thanks for the PR and all improvements related to mongodb! would you mind fixing the mongo test errors for the broken build? |
* Use the official MongoDB Go Driver rather than mgo * Add timeseries-collection, retryable-writes, and ordered-inserts options
gregorynoma
force-pushed
the
pull-request
branch
from
b3f59f5
to
ec9f163
Compare
|
Hi @jonatas sorry about that, should be fixed now. |
jonatas
requested changes
Jan 19, 2022
There was a problem hiding this comment.
Thanks for all the fixes! Just a few things that can make the project easy to maintain:
- I was trying to test locally and, I see the project has the full_cycle_minitest folder with one script for each database. Maybe that would be great to have it for mongo too. It can help me to review and understand some basic scenarios from a short benchmark example.
- I also see an opportunity to leave some sample-configs with the same purpose but YAML files.
| // GroupByOrderByLimit populates a query.Query that has a time WHERE clause, that groups by a | ||
| // truncated date, orders by that date, and takes a limit, e.g. in pseudo-SQL: | ||
| // | ||
| // SELECT minute, MAX(cpu) FROM cpu |
There was a problem hiding this comment.
In the implementation, the column name usage_user. Should we fix the docs too?
Suggested change
| // SELECT minute, MAX(cpu) FROM cpu | |
| // SELECT minute, MAX(usage_user) FROM cpu |
|
Hey @jonatas, I added those files you suggested as well as incorporated some additional changes we made since the original PR! |
|
Thank you, @gregorynoma! I'll review it again! |
|
Hi All, Any updates on this? Looking forward to the benchmark results! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 adds support for MongoDB time-series collections, which are available starting in MongoDB 5.0. The changes include: