This is the major part of #7470. Additional clean ups (e.g. to remove
the actual types from `data_types`) will follow.
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
A short description on the FlightFrameEncodeRecorder that helps people
understand exactly what the spans cover - it's likely people will wind
up looking at this code after debugging an issue in a trace, so lets
make sure we give them as much helpful context as possible!
* test: integration test for tracing of queries to the ingester
* chore: add FlightFrameEncodeRecorder to record spans per each polling result
* refactor(trace): impl TraceCollector for Arc
Allow any Arc-wrapped TraceCollector implementation to be used as a
TraceCollector. This avoids needing to as_any() and downcast later.
* test: assert FlightFrameEncodeRecorder trace spans
This test exercises the FlightDataEncoder wrapped with the trace
decorator (FlightFrameEncodeRecorder) when executing against a data
source that yields data after varying numbers of Stream polls.
This test passing will validate the FlightFrameEncodeRecorder correctly
instruments the amount of time a client spends waiting on the
FlightDataEncoder to acquire or encode a protocol frame, but also
ensures the decorator correctly accounts for varying behaviours allowed
through the Stream abstraction. It does this by simulating a data source
that is not always immediately ready to provide data, such as a buffer
wrapped in a contended async mutex.
* refactor: move tracing decorator into separate mod
* fix: record spans
* refactor(test): update test
The frame encoder is not one-to-one - it emits two frames for the first
data payload, a schema and a payload. This commit updates the test to
account for it!
* refactor: remove unneeded mut ref, and use enum state method which panics when in a (should be unreachable) state
* chore: add more docs to FlightFrameEncodeRecorder and related
---------
Co-authored-by: Dom Dwyer <dom@itsallbroken.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* fix(ingester): re-transmit schema over flight if it changes
Fixes https://github.com/influxdata/idpe/issues/17408 .
So a `[Sendable]RecordBatchStream` contains `RecordBatch`es of the SAME
schema. When the ingester crafts a response for a specific partition,
this is also almost always the case however when there's a persist job
running (I think) it may have multiple snapshots for a partition. These
snapshots may have different schemas (since the ingester only creates
columns if the contain any data). Now the current implementation munches
all these snapshots into a single stream, and hands them over to arrow
flight which has a high-perf encode routine (i.e. it does not re-check
every single schema) so it sends the schema once and then sends the data
for every batch (the data only, schema data is NOT repeated). On the
receiver side (= querier) we decode that data and get confused why on
earth some batches have a different column count compared to the schema.
For the OG ingester I carefully crafted the response to ensure that we
do not run into this problem, but apparently a number of rewrites and
refactors broke that. So here is the fix:
- remove the stream that isn't really as stream (and cannot error)
- for each partition go over the `RecordBatch`es and chunk them
according to the schema (because this check is likely cheaper than
re-transmitting the schema for every `RecordBatch`)
- adjust a bunch of testing code to cope with this
* refactor: nicify code
* test: adjust test
This commit fixes loads of crates (47!) had unused dependencies, or
mis-configured dependencies (test deps as normal deps).
I added the "unused_crate_dependencies" to all crates to help prevent
this mess from growing again!
https://doc.rust-lang.org/beta/nightly-rustc/rustc_lint_defs/builtin/static.UNUSED_CRATE_DEPENDENCIES.html
This has the minor downside of false-positives when specifying
dev-dependencies for test/bench binaries - these are files in /test or
/benches (not normal tests). This commit includes a workaround,
importing them in lib.rs (gated by a feature flag). I think the
trade-off of better dependency management is worth it!
Adds a test that asserts (manually triggered) persistence generates a
file, uploads it to object storage, inserts metadata into the catalog,
and emits various persistence metrics.
Implements a PersistCompletionObserver that records various attributes
of the generated and persisted Parquet file as histogram metrics to
capture the distribution of values:
* File size
* Row count
* Column count
* Time range of data (max - min timestamp)
These metrics will give us insight into the generated files instead of
relying on intuition when tuning various configuration parameters.
* chore: Upgrade to Rust 1.68
* fix: Remove unnecessary into_iter, thanks Clippy!
* fix: Use the size of the type, not a reference to the type... oops.
Thanks clippy!
* fix: Return block directly instead of creating a variable
Thanks clippy!
---------
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Andrew Lamb
dependabot[bot]
wiedld
Marco Neumann
Dom
Andrew Lamb
Carol (Nichols || Goulding)
Kaya Gökalp
Carol (Nichols || Goulding)