Fixes on the path to closing multiple axes#123
Conversation
…tely seen in the update: thread 'main' (66669) panicked at /home/max/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/opentelemetry-jaeger-0.19.0/src/exporter/runtime.rs:35:12: Registering a blocking socket with the tokio runtime is unsupported. If you wish to do anyways, please add `--cfg tokio_allow_from_blocking_fd` to your RUSTFLAGS. See github.com/tokio-rs/tokio/issues/7172 for details. Updating RUSTFLAGS seeing as Reductionist has always worked this way, needs to be addressed independently.
…ficient serialization
maxstack
force-pushed
the
fixes-on-the-path-to-closing-multiple-axes
branch
from
056a158 to
e9a4de0
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds request-controlled options to encode large shape and/or count vectors as compact byte strings in the CBOR response (to reduce Python client deserialization memory), updates related tests/bench fixtures and documentation, and introduces a Tokio workaround via a global rustc cfg flag.
Changes:
- Add
option_shape_as_bytes/option_count_as_bytestoRequestDataand thread these options through numeric operations into the response. - Extend CBOR response serialization to include
shape_as_bytes/count_as_bytes, plus unit tests for the new behavior. - Update deployment/API docs and local storage startup script; add
.cargo/config.tomlrustflags workaround for a Tokio blocking-FD assertion.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test_utils.rs | Updates test request fixtures to include the new optional request flags. |
| src/operations.rs | Passes the new request options into Response::new(...) for several operations. |
| src/operation.rs | Updates operation tests to match the new Response::new(...) signature. |
| src/models.rs | Adds request flags, extends Response, and implements byte-encoding in CBORResponse with new tests. |
| scripts/storage-start | Generates an htpasswd file for local nginx basic auth. |
| docs/deployment.md | Updates chunk cache key docs to reflect %url-based tokenization. |
| docs/api.md | Documents the new request options. |
| benches/operations.rs | Updates bench request fixture to include new optional request flags. |
| benches/byte_order.rs | Updates bench request fixture to include new optional request flags. |
| .cargo/config.toml | Adds a global tokio_allow_from_blocking_fd cfg via rustflags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Bump lz4_flex from 0.12.0 to 0.12.1 - Bump quinn-proto from 0.11.13 to 0.11.14
maxstack
force-pushed
the
fixes-on-the-path-to-closing-multiple-axes
branch
from
266e9af to
ba75577
Compare
There was a problem hiding this comment.
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Hopefully close the reduction over multiple axes work:
countreturned gets big and as it's a vector of int64 a python client will deserialise the CBOR response into a list using lots of memory in the processcountto be converted and sent down the wire asbytes, easily deserialised and converted into a list only if neededFixes to documentation
Workaround for an assert recently added to
tokiothat panics if we have a blocking socketUpdate dependencies to appease dependabot: