-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix: backport improvements to ReadTags pushdowns from 2.x #21160
Conversation
8009fe1
to
f676bfa
Compare
@@ -16,6 +16,7 @@ v1.9.0 [unreleased] | |||
- [#21074](https://github.com/influxdata/influxdb/pull/21074): feat: upgrade to flux 0.111.0 | |||
- [#21100](https://github.com/influxdata/influxdb/pull/21100): feat: add memory and concurrency limits in flux controller | |||
- [#21108](https://github.com/influxdata/influxdb/pull/21108): feat: make flux controller limits configurable | |||
- [#21124](https://github.com/influxdata/influxdb/pull/21124): feat: implement rewrite rules for window and bare aggregates |
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.
Forgot to update the CHANGELOG in the last PR, catching up here
@@ -293,16 +344,6 @@ func (s *Store) TagKeys(ctx context.Context, req *datatypes.TagKeysRequest) (cur | |||
} | |||
|
|||
func (s *Store) TagValues(ctx context.Context, req *datatypes.TagValuesRequest) (cursors.StringIterator, error) { | |||
if tagKey, ok := measurementRemap[req.TagKey]; ok { |
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.
This looks like a good optimisation to keep?
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 added below, starting here: https://github.com/influxdata/influxdb/pull/21160/files#diff-6d1e664d01750fd2f45528ac06c5a98288a4d14b20e5fe3b36b759eedc8b4cc9R387
services/storage/store.go
Outdated
type MeasurementNamesRequest struct { | ||
MeasurementsSource *types.Any | ||
Predicate *datatypes.Predicate | ||
func (s *Store) MeasurementNames(ctx context.Context, mqAttrs *metaqueryAttributes) (cursors.StringIterator, error) { |
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.
Changing this will also require changing the Enterprise RPC requests
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.
Good point... should metaqueryAttributes
be MetaqueryAttributes
for it to be accessible from plutonium?
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.
Hrm, it looks like enterprise contains a copy-paste of the old code, with no RPC for the Flux meta queries. Is that expected? Disregard, the line that calls into the TSDB store in OSS is replaced in Enterprise
Closing this as it will be too difficult to make it work for Enterprise. We could consider re-working it in 1.9.1 |
Part of #21069
Backports #19856 and #19894