The controller now supports setting an initial memory limit and setting
a maximum amount of memory that the controller may use separately from
the memory quota per query and the concurrency quota.
This allows the controller to increase the concurrency quota to a larger
number while setting the maximum amount of memory to a lower amount than
would be required for all queries to use 100% of their allowable memory.
Functionally, this means that a query will have a soft limit for an
initial memory byte quota that a query is guaranteed to have, a shared
pool that it is allowed access to in the case it uses more, and a hard
limit that no query may exceed to prevent runaway queries from taking
over the entire pool.
This change is completely backwards compatible with older configurations
as the new options will default to values that mimic the old behavior
where a query is allocated the full amount of its memory quota and the
maximum amount of memory is based on the concurrency quota and this
maximum memory quota.
In addition to the above, this also fixes a bug in the controller that
allowed it to run more than its concurrency as executing queries. This
happened when the results had finished being sent by the executor, but
the query had not yet been read and/or serialized. The executor would be
freed up and would take the next query even though the previous query
hadn't yet been finalized with `Done()`.
The QueryServiceProxyBridge would not check for errors properly because
it would return any error encountered when running the query as a read
error on the `io.Reader`. This made it so that the csv decoder could not
identify if the error was related to the query or if it was related to
reading. The csv decoder needed to tell the difference because an error
with reading from the `io.Reader` needs to be returned as a decoder
error while an error from the query needs to be returned as-is.
Instead of adapting the csv decoder to do that, we instead lazily
initialize the result iterator when `More()` is called and call `Peek()`
on the reader. If no bytes can be read, we assume this was an error
while executing the query and return it as such. If we are able to read
at least one byte, we decode it through the csv decoder.
The http error schema has been changed to simplify the outward facing
API. The `op` and `error` attributes have been dropped because they
confused people. The `error` attribute will likely be readded in some
form in the future, but only as additional context and will not be
required or even suggested for the UI to use.
Errors are now output differently both when they are serialized to JSON
and when they are output as strings. The `op` is no longer used if it is
present. It will only appear as an optional attribute if at all. The
`message` attribute for an error is always output and it will be the
prefix for any nested error. When this is serialized to JSON, the
message is automatically flattened so a nested error such as:
influxdb.Error{
Msg: errors.New("something bad happened"),
Err: io.EOF,
}
This would be written to the message as:
something bad happened: EOF
This matches a developers expectations much more easily as most
programmers assume that wrapping an error will act as a prefix for the
inner error.
This is flattened when written out to HTTP in order to make this logic
immaterial to a frontend developer.
The code is still present and plays an important role in categorizing
the error type. On the other hand, the code will not be output as part
of the message as it commonly plays a redundant and confusing role when
humans read it. The human readable message usually gives more context
and a message like with the code acting as a prefix is generally not
desired. But, the code plays a very important role in helping to
identify categories of errors and so it is very important as part of the
return response.
We are planning to change the allocator interface within flux to use the
arrow allocator. To make the release easier, this updates the test in
advance to use the arrow allocator instead of the to be changed memory
allocator interface from flux.
Writes directly to a PointsWriter require the tag key, value pairs
are sorted in lexicographically ascending order. This commit uses
new API from the `models` package to ensure this invariant is
maintained.
The `v1.databases()` call did not correctly filter buckets based on
auth. Fortunately, it did not cause any improper permissions such as
allowing a person to see buckets that they had no read access to.
The error instead was that if a user did not have read access to one of
the buckets that was returned, the entire command would fail rather than
filter out the bucket that didn't have permissions.
This changes it so that if the user doesn't gets an unauthorized error
when accessing a bucket, it will filter it from the list instead of
failing. It also changes it so the error message is marked as
`ENotFound` instead of as an internal error.
The secret service is tested by creating a secret and then attempting to
use it in a flux query. There is one test where accessing the secret
should work and one where it should return that the action is forbidden.
This change makes it so that if an org or orgID are missing on calls to the `to` function
that the orgID is retrieved from the request context.
This is consistent with how `from` works.
The secret service is wrapped to be compatible with the flux interface
to the secret service.
The organization id is not part of the flux interface so this code
extracts the organization id from the query context so that it can lookup
a secret within the organization.
The `exists` operator now gets pushed down to storage correctly. If
`exists` is used on a tag, then it will be rewritten to `tag != ""`
which is how storage defines if a tag exists. If `not exists` is used,
then it will use `tag == ""` which is how you would query storage for
only if a tag doesn't exist.
The `tag == ""` and `tag != ""` are different. For `tag == ""`, the
predicate is impossible for the storage layer to return true with.
Ideally, we would just rewrite this to return nothing and we wouldn't
bother with even querying storage. Instead, we just do not rewrite this
predicate because it cannot be rewritten to make sense with storage. If
we see `tag != ""`, it is the only one that can be passed through as-is
because `tag != ""` returns the same values as `exists tag`. It will
return true for every non-null value.
If we handle the flux errors in the query controller, it makes it so we
are handling the errors in the location where the happen rather than at
a layer further up the stack.
This should simplify it so the errors are handled in this single
location instead.
In the QueueSize test, it was possible that after the `done` channel was
closed, one of the queries from the queue would begin executing. If all
three began executing before the shutdown was done, the third would
block on attempting to send a value to the `executing` channel and it
would never finish so the controller would report that shutdown failed.
This increases the queue size to a combination of the concurrency quota
and the queue size so all of the started queries will never block when
sending a signal to the executing channel.
The storage table reader will now work correctly when there are multiple
outputs. The table interface now implements the new table and column
reader interfaces and works properly with `execute.CopyTable`. The
source uses `execute.CopyTable` to buffer the table in memory when there
are multiple output transformations.
The controller implementation is primarily used by influxdb so it
shouldn't be part of the flux repository. This copies the code from flux
to influxdb so it can be removed from the next flux release.
The `databases()` function was moved into the `influxdata/influxdb/v1`
package and so it wouldn't work anymore. The `percentile()` call was
changed to `quantile()` and the argument was changed to `q`. This also
updates `median()` to use `median()` since we now produce AST's and not
the spec so we can use the `median()` definition instead.
The RPC call should translate `_measurement` and `_field` to their
proper shortened byte strings when requesting the tag values.
This also fixes the planner rewrites to return the root node even when
no rewrite happened as this is required by the planner.
If a pattern is seen that matches the `v1.tagValues(...)` call, then it
will be replaced with a direct RPC call to read the tag values for the
selected tag key which should be better optimized than reading from the
storage engine tsm1 files.
If a pattern is seen that matches reading the tag keys, it will be
replaced with a direct RPC call to read the tag keys which should be
better optimized than reading from the storage engine tsm1 files.
The ProxyQueryServiceAsyncBridge was not returning statistics when there
was an encoder error. Because the encoder was just writing to an
io.Writer, it was possible that a remote disconnect could happen and
statistics could not be reported.
BucketsAccessed doesn't work currently with a private flux.Spec.
See this issue: https://github.com/influxdata/influxdb/issues/13278
This set of changes just allows code to compile until #13278 is fixed.
Note that preauthorization is not working in the meantime.
Fixes#13275.
It is no longer necessary for the query logging to be a bridge as the
stats are available for consumption from the ProxyQueryService.
This change changes the logging bridge to directly implement the proxy
query service instead of implementing a bridge.
* deps: update flux-staging to master of both influxdb and flux (#12698)
* test(stdlib): skip failing window test due to new trigger optimization (#12708)
* chore: update Flux to v0.22.0
The asyncStatsResultIterator used inside QueryServiceProxyBridge assumed
that Release would only be called once. The godoc for ResultIterator
specifies that it is safe to call Release multiple times. Now, you can
do that without causing Release to block indefinitely.
The statistics are only finalized after release is called. Defer a call
to release to ensure they are released, but explicitly release on
success to ensure that the statistics are finalized from all sources
before returning them.
This refactors everything to generate and use a flux AST when
transpiling an influxql query. This also updates the spectests so they
use flux instead of writing out the AST and compare the resulting
AST's.
When trying to create a task that writes to an unknown bucket,
previously the error message would resemble:
could not find bucket {<nil> 0xc0000dfcc0 <nil> 0xc0000dfce0}: <not found> bucket not found
Now print something human-readable:
could not find write bucket with filter: [Bucket Name: b, Org Name: a]: <not found> bucket not found
filter out resources that have mission IDs
fix(influxdb): simplify auth check in PermissionAllowed
review(platform): update as noted in review
fix(influxdb): ensure permission has valid org id
I did this with a dumb editor macro, so some comments changed too.
Also rename root package from platform to influxdb.
In interest of minimizing risk, anyone importing the root package has
now aliased it to "platform" so that no changes beyond imports were
necessary in those files.
Lastly, replace the old platform module to local path /dev/null so that
nobody can accidentally reintroduce a platform dependency while
migrating platform code to influxdb.
all tests use a unique bucket based on the test file name. copied all tests over from flux repo
the tests are currently disabled due to engine consistency issues: https://github.com/influxdata/flux/issues/613
* refactor(cmd/influxd): move driver code for influxd main package to sub-package so it can be reused.
* chore(query/influxql): moved query_test.go and requisite files to the influxql dir to be closer to the code it tests. (#2013)
A standard Makefile is used now in all subdirs that run go generate.
Make will only generate the file if its source files changed.
The checkgenerate target runs clean to ensure all targets a generated
fresh.
The table interface was modified to expose the arrow buffers. The
storage table has now been converted to use this interface with the same
fixes so that it exposes arrow buffers.
The influxql package has also been updated to use the `DoArrow` method
from the `flux.Table` interface.
The flux query controller was updated to include a Shutdown method a
while ago. Explicitly handle query controller creation and shutdown
where applicable.
In influxd, this ensures that outstanding queries are handled before the
process dies. In tests, this ensures that query controller goroutines
aren't leaked, which drastically simplifies reading full stack traces.
This change also registers query controller metrics with the prometheus
registry in influxd.
feat(platform): add ToPermissions method to user resource mapping
The ToPermissions method returns a set of permissions that is granted
via a user resource mapping.
feat(bolt): resolve sessions permissions on lookup
feat(http): use authorizer instead of authorization service for write api
feat(bolt): create user resource mappings for org users in bucket create
feat(bolt): create user resource mapping for first org/user
fix(platform): use authorizer for query endpoint instead of authorization
test(http): use cmp instead of reflect for decode test
* Add default column to databases to support `show retention policies`
* add unit test for show retention policies
* update transpiler readme for show databases, show retention policies
The generate commands have been modified to take advantage of the new
functionality in Go 1.11 that allows `go run` to execute a package
instead of individual files.
This functionality combined with Go modules allows us to execute a
package directly out of our pinned dependencies rather than accidentally
picking up another binary outside of the build environment.
This also simplifies the Makefile because they no longer have to be
responsible for installing the correct tooling since the Go command
takes care of that logic. It also makes it so that the Makefiles with
file generation can now be invoked from their appropriate subdirectories
so they are contained within the directory itself rather than relying on
values in the top level Makefile.
It is now possible to generate all files within this project by using:
go generate ./...
Or the Makefile can continue to be used.
This commit also copies over the special copy of `tmpl` that the storage
engine uses within the influxdb repository. It was never copied over so
using `go generate` on these packages did not work.
The pb package was only referenced in cmd/influx/query.go, but in
dead code, since it uses the same machinery as the repl, which goes
through the HTTP endpoints, rather than the gRPC endpoints.
We reorganized the functions in flux to have the structure:
/functions
/inputs
/transformations
/outputs
this PR catches up platform to work with the new package layout.
As a separate refactoring issue, we should discuss:
from(bucket: ) should migrate from flux --> platform
to_http and to_kafka should migrate from platform --> flux
It creates a simple client that follows the QueryService interface and
uses the `influxql.Compiler` type to determine where the query should be
routed to and to return the query as a `flux.ResultIterator`.
This will be useful for replaying transpiler queries against influxdb
1.x servers to verify correctness.
This iterface is supposed to be something that both sessions and
authorizations can share so that other components can authorize requests
as they see fit.
The `-i` flag causes a problem when a cross compilation happens because
it attempts to build files in GOROOT which is located in a directory
that is not writable by the build user. This same problem exists for
when the `-tags` are changed like adding `-tags assets` in the `all`
build that runs on the master branch.
This removes `-i` because, in the latest go release, the `-i` flag
doesn't actually do anything useful since there is another build cache
that the results are placed in regardless of whether `-i` is used.
This fixes the build on master so that `make all` and `make nightly`
will function properly.
This updates the dependency manager to use go modules instead of dep so
that we can remove dep as a dependency and we can begin using and giving
feedback on modules within this project. It should simplify dependency
management and make it faster to run the builds too.
The big change is updating the various Makefiles to stop relying on the
vendor directory because it no longer exists. This change creates a
`tools.go` file with the revisions of the tools we use that are currently
in the `required` section of `Gopkg.toml`. We are currently in other
discussions to modify how we handle tool dependency management, but this
change does not change that.
The transpiler will normalize the `_time` column by dropping any
existing time column and then duplicating `_start` when the query is an
aggregate type.
This works for the selectors because they did not normalize their
`_time` column at all and, while the aggregates did normalize their
`_time` column, we have made the decision to remove that functionality
and have aggregates not set a `_time` column at all.
Instead of generating multiple cursors, a pivot is used to join fields
within the same series.
This should be easier than generating a new cursor for everything.
WIP: saving state reword
feat(query/influxql): implement query.ResultIterator directly on response
review(influxql): add ResponseIterator that implement query.ResultIterator
review(influxql): update code is response to review
The previous default was just to have no limit at all. This adds a
configuration option to the planner so a static value can be set for the
memory limit on each individual query.
Using query request struct to query resources
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Use query.ProxyRequest instead query.Request
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Proxy request from idpd
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Comments about the desired results
Signed-off-by: Lorenzo Fontana <lo@linux.com>
V1 endpoints working with flux
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Influxql working for v1
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-authored-by: Michael De Sa <mjdesa@gmail.com>
V2 influxql query endpoint working
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-authored-by: Michael De Sa <mjdesa@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
V2 Flux compiler support
Co-authored-by: Michael De Sa <mjdesa@gmail.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Improve comments in bolt sources and give error on self
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-authored-by: Michael De Sa <mjdesa@gmail.com>
Review tests failing
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-authored-by: Michael De Sa <mjdesa@gmail.com>
Avoid type casts for compiler types
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-authored-by: Michael De Sa <mjdesa@gmail.com>
Using nil instead of dbrp mapping service for influxql v1
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Check if compiler types are valid for influxql
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Organization as query param in the flux external handler
Signed-off-by: Lorenzo Fontana <lo@linux.com>
feat(http): update swagger documentation for flux query endpoint
feat(http): document query endpoint design
The code documented does not currently work. It is indended that this
will be implemented in follow up PRs.
feat(platform): move source to platform package
The source Query endpoint implements what's in the query swagger docs
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Co-authored-by: Michael De Sa <mjdesa@gmail.com>
feat(platform): allow for encoding and decoding of csv dialects
feat(platform): specify dialect in flux page
Co-authored-by: Andrew Watkins <andrew.watkinz@gmail.com>
Co-authored-by: Michael Desa <mjdesa@gmail.com>
The query statistics would only be read after the cancel call so we need
to wait for that before attempting to read the statistics rather than
attempting to read them immediately after the result is returned (before
it is read).
* Added default signature generators for Selector and Aggregate configs and update the functions that need them
* fix to percentile to collect the correct arguments for both aggregate and selector
The REPL's use of the interpreter was causing it to not get the builtins
defined in builtin scripts. For example the `top` function was missing.
This change fixes the issues by ensuring the builtins are only evaluated
once and that there is only one way to get the query Interpreter that is
guaranteed to have the proper builtins.
Jonathan A. Sternberg
Lyon Hill
Christopher Wolff
Lorenzo Affetti
Stuart Carnie
Nathaniel Cook
Adam
docmerlin (j. Emrys Landivar)
George
roshie548
Adam
jlapacik
Ben Johnson
Kelvin Wang
Todd Persen
Mark Rushakoff
Jacob Marble
kelwang
zhulongcheng
Jacob Marble
Michael Desa
Mark Rushakoff
Andrew Watkins
Edd Robinson
Chris Goller
Leonardo Di Donato
Jeff Wendling
Brandon Farmer
Adam Perlin