docs(ingress): make QWP/WebSocket the primary documented transport#145
Open
jerrinot wants to merge 22 commits into
Open
docs(ingress): make QWP/WebSocket the primary documented transport#145jerrinot wants to merge 22 commits into
jerrinot wants to merge 22 commits into
Conversation
The published rustdoc landing pages on docs.rs/questdb-rs/ still framed
ILP as the primary transport and did not document QWP/WebSocket as a
first-class user-facing surface. This commit reorganises the user-facing
documentation so that QWP/WebSocket is the recommended path and ILP is
relegated to a clearly-marked legacy section. No behaviour changes.
questdb-rs/README.md
- Transports section leads with ws::/wss:: (QWP/WebSocket) and demotes
http::/https:: and tcp::/tcps:: as legacy. QWP/UDP is not advertised.
- Quick Start switched to a ws:: example with close_drain.
- Protocol Versions clarified: QWP carries types natively; the ILP
protocol_version mechanism applies only to ILP/HTTP and ILP/TCP.
- Examples table relabels existing examples as ILP/legacy and adds the
QWP/WebSocket benchmark.
- Default features list trimmed (no QWP/UDP advertised).
- All GitHub and docs.rs URLs pinned to the 6.1.0 release tag so the
README inside a published crate keeps stable links.
questdb-rs/src/ingress/mod.md (~449 lines, down from ~470)
- Lead paragraph rewritten to recommend QWP/WebSocket and explain the
legacy ILP positioning up front.
- Configuration string section uses ws::/wss:: throughout and links to
questdb.io/docs/connect/clients/connect-string/ for the full key
reference rather than enumerating keys inline.
- "Don't Forget to Flush" reframed: the client has no auto-flush on
any transport (auto_flush_rows / auto_flush_bytes /
auto_flush_interval all rejected; auto_flush=off accepted as a
compatibility no-op). Buffer.len()-based flush-timing guidance
restored.
- "Error Handling on QWP/WebSocket" kept concise; protocol-level
detail (Halt vs DropAndContinue framing, FSN watermarks, multi-host
failover, store-and-forward, durable acknowledgement) is summarised
in a single section that lists the Rust API entry points and links
out to questdb.io/docs/high-availability/ and the C/C++ client
doc for depth.
- Authentication section calls out OIDC / mTLS / token-rotation as
unsupported by this client, with workarounds.
- TLS, close_drain, health-check, buffer-API sub-sections preserved
with minor reframing for a QWP-first reader.
- Array and decimal sections note that QWP carries these types
natively (no protocol_version handshake required).
- Legacy ILP Transports section at the bottom covers ILP/HTTP and
ILP/TCP for backwards-compatibility readers, with an explicit
legacy admonition.
- The implicit-promotion claim about initial_connect_retry (which is
a Java-client behaviour, not present in this crate) has been
removed.
questdb-rs/src/ingress/sender.rs
- Sender::from_conf documents transports in QWP-first order and drops
the qwpudp entry.
- Sender::from_conf and Sender::from_env note that both TCP and
QWP/WebSocket complete authentication and TLS synchronously before
returning.
- Sender::flush recommendation no longer reads "HTTP first, TCP for
high rate"; QWP/WebSocket is the default recommendation, framed
around structured error reporting and durability rather than raw
throughput.
- Sender::must_close description reordered so QWP/WebSocket is
primary; ILP/TCP follows.
questdb-rs/src/ingress/buffer.rs
- Buffer struct-level doc no longer claims dispatch is "ILP or
QWP/UDP" only; it now correctly covers QWP/WebSocket as well.
Validation: cargo doc --no-deps builds clean with no warnings;
cargo test --doc passes all 50 doctests (including the 13 new code
blocks in mod.md and the new README example).
Untouched in this commit: doc/QWP_UPSTREAM_PR_NOTES.md was already
untracked and remains so. Per-method buffer.rs doc comments that say
"For ILP buffers ... For QWP/UDP buffers ..." are factually correct
descriptions of internal dispatch and were left as-is.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR repositions the QuestDB Rust client ( ChangesQWP/WebSocket Client Documentation and Examples Pivot
Sequence DiagramsequenceDiagram
participant User
participant Sender
participant Buffer
participant QWP as QWP/WebSocket
participant Server
User->>Sender: from_conf(connect_string)
Sender->>QWP: establish connection (TLS/auth handshake)
User->>Buffer: table() . symbol() . column_*() . at_now()
Buffer->>Sender: flush()
Sender->>QWP: send buffered rows
QWP->>Server: publish data
Server-->>QWP: async error or ack
Sender->>QWP: poll_qwp_ws_error or registered error callback
User->>Sender: close_drain()
Sender->>QWP: await pending acks
QWP-->>Sender: confirmed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Outbound documentation links were pointing at questdb.io/docs/..., which
301-redirects to questdb.com/docs/.... One reference also targeted the
C/C++ client documentation page instead of the Rust one. This commit:
- Migrates every questdb.io/docs/ URL in mod.md and README.md to
questdb.com/docs/ so readers do not pay a redirect hop and the
canonical domain is what shows in the address bar. Slack and
community/marketing URLs stay on .io (different services).
- Retargets the async-error-handling link in mod.md from
/docs/connect/clients/c-and-cpp/#asynchronous-error-handling to
/docs/connect/clients/rust/#asynchronous-error-handling. The Rust
client doc page exists with the same anchor and is the right
deep-link from a Rust rustdoc.
- Adds a one-line CTA in mod.md immediately after the Hello World,
pointing readers at the full Rust client documentation page on
questdb.com for the topics that the trimmed rustdoc no longer
covers in depth (failover, store-and-forward, durable ACK,
troubleshooting).
- Adds a parallel CTA paragraph to the README's Docs section so the
crates.io landing surface also points at the Rust client guide,
in addition to the docs.rs API reference.
No code changes; all 50 doctests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The section led with "QWP auto-negotiates the protocol version" — i.e. "nothing to configure" — and then spent six lines plus a table walking a new reader through legacy ILP version handshakes that they do not need to think about to ingest their first row. Net effect was to slow down the new-reader path with legacy trivia. The legacy ILP version nuances are still documented in ingress/mod.md under the ## ILP/HTTP and ## ILP/TCP subsections, which is the right context: a reader who is already choosing a legacy transport finds the protocol_version detail where they need it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Examples table listed eight files but only one of them (qwp_ws_unified_sfa_bench.rs) actually exercised QWP — the other seven were ILP-based and being advertised on a page that otherwise demotes ILP as legacy. Trimmed to the single QWP example so the section is consistent with the rest of the README. The legacy ILP examples are still in the questdb-rs/examples/ directory for readers who want to look; they are simply no longer advertised on the crates.io landing surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Examples table on the crates.io landing surface only had one QWP
entry (a throughput benchmark), which is too specialised to serve as
the first example a new reader copies. This commit adds three more
QWP/WebSocket examples and lists them in the README.
examples/qwp_ws_basic.rs -- minimal ingest: from_conf,
one row, flush, close_drain.
Mirrors examples/basic.rs.
examples/qwp_ws_failover.rs -- multi-host addr= list with
sf_dir store-and-forward
and sender_id; per-iteration
flush with must_close check.
examples/qwp_ws_error_handling.rs -- both async-error styles:
poll_qwp_ws_error loop and
SenderBuilder::qwp_ws_error_handler
callback. Includes a
qwp_ws_errors_dropped check.
Each example is added to Cargo.toml with
`required-features = ["sync-sender-qwp-ws"]` matching the convention
already used by `qwp_ws_unified_sfa_bench`.
README's Examples section restructured as a four-row table that lists
all four QWP examples and shows the cargo run incantation, replacing
the single-link sentence shape from the previous commit.
Validation: `cargo build --examples` builds all four QWP examples
cleanly (plus the existing ILP-feature-gated ones).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The crate index page rendered from README.md (file:///.../target/doc/ questdb/index.html, docs.rs/questdb-rs/<version>/questdb/) led with a generic "QuestDB Database docs" bullet and the rustdoc itself. There was no prominent index-page link to the Rust-specific guide on questdb.com/docs/connect/clients/rust/ -- it was only reachable from the Docs section further down. Promote the Rust client documentation page to the first top-of-page bullet so a reader landing on docs.rs gets one click to the canonical Rust client guide. The rustdoc API reference moves to second, and the generic database docs to third. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A QWP-first reader hitting either the rustdoc landing page or the ingress module page saw an "InfluxDB Line Protocol transports remain supported but are not recommended" sentence within the first 15 lines, before any QWP example had been shown. The Legacy ILP Transports section in mod.md (and the Transports section in README.md) already carries that information in its proper context. Dropping the intro mention keeps the first impression on QWP. The mid-page "to inspect ILP buffer bytes, call buffer.as_bytes()" paragraph that sat in the QWP Troubleshooting section is also removed; it interrupted the QWP narrative with an ILP-specific mechanic. ILP buffer inspection remains discoverable from the Buffer::as_bytes rustdoc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The SenderBuilder struct doc and several of its per-method comments
still framed ILP as the recommended path, listed only ILP examples, or
omitted QWP/WebSocket from per-transport descriptions:
- SenderBuilder struct doc: three doctests all using https::/Protocol::Http;
no ws:: example. Trimmed to one ws:: doctest plus a link to
questdb.com/docs/connect/clients/rust/ for the full configuration
reference.
- SenderBuilder::from_conf: the "We recommend HTTP" paragraph and the
enumeration of QWP-only connect-string keys collapsed into a short
paragraph linking to the connect-string reference on questdb.com.
- SenderBuilder::new: dropped the "using ILP over the specified
protocol" claim (wrong since Protocol::QwpWs/Wss/Udp exist) and the
feature-gated ILP-only example.
- SenderBuilder::username/password/token: rewrote to name QWP/WebSocket
and ILP/HTTP for basic and bearer-token auth (both transports
accept these per accepts_http_auth in ingress.rs:316), with ECDSA
framing scoped to ILP/TCP.
- SenderBuilder::protocol_version: clarified that this knob applies
only to ILP transports; QWP transports negotiate during handshake
and ignore the setting.
- SenderBuilder::max_name_len, Sender::max_name_len: clarified that
the server-side pickup is ILP/HTTP-with-V2-specific; other
transports use the configured/default value.
- SenderBuilder::build: noted that QWP/WebSocket likewise establishes
its connection synchronously, matching the existing TCP note.
- Sender::flush_and_keep: now cross-references Sender::flush for the
per-transport semantics (QWP/WebSocket local-publish, HTTP retry,
TCP fire-and-forget) instead of saying nothing.
The general principle for this pass: keep the docstring short, name
the transports involved if they materially differ, and link to the
QuestDB documentation site for the protocol-level depth.
cargo test --doc passes (47 doctests; the previous 50 included three
SenderBuilder struct-doc examples that have been collapsed).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Six per-method comments on Buffer either omitted the QWP/WebSocket
branch entirely or led with ILP. Two were factually wrong by omission:
- Buffer::transactional claimed only "QWP/UDP" returns false; the
QWP/WS branch also returns false unconditionally.
- Buffer::as_bytes said "QWP/UDP returns an empty slice"; QWP/WS does
the same.
The other four (Buffer::protocol_version, Buffer::reserve, Buffer::len,
Buffer::capacity) used "For ILP ... For QWP/UDP ..." wording with
QWP/WebSocket either missing or trailing.
Standardised on "QWP buffers (UDP and WebSocket)" where the two QWP
branches share semantics; this is the actual code shape (both Qwp(_)
and QwpWs(_) match arms return the same values for these methods).
Where the WS branch genuinely diverges, the divergence is now stated
explicitly.
Also fixed the misleading "QWP datagram version" phrasing on
Buffer::protocol_version (WebSocket frames are not datagrams) to
"QWP wire-format version".
cargo test --doc still passes (47 / 47).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The published rustdoc on `ingress/mod.md` advertises that this client has no auto-flush on any transport, and that the `auto_flush_rows`, `auto_flush_bytes`, and `auto_flush_interval` keys in the connect string are rejected. `validate_auto_flush_params` only iterates over the first two; unknown keys (including `auto_flush_interval`) fall through to the silent-accept branch in the main parser, so a user who sets `auto_flush_interval=1000;` gets no error and no flushing. Add `auto_flush_interval` to the rejection list so the validator matches the documented behaviour, with a parallel `auto_flush_interval_unsupported` test next to the existing rows/bytes cases. Also folds in two unrelated minor tidies that were sitting in the tree: - ingress.rs: reorder the `SenderBuilder::max_buf_size` doc-comment bullets so QWP/WebSocket is listed first, matching the QWP-first orientation of recent docs commits. - examples/qwp_ws_basic.rs: rustfmt wrap of an `unwrap_or_else` call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
questdb-rs/src/ingress/buffer/ilp.rs had eight `questdb.io/docs/...` links in /// comments inside the private `ilp` module. The module is not pub-exposed (rendered HTML is reachable only via the rustdoc "source" link), so this is a code-hygiene migration rather than a user-facing fix. The questdb.io domain 301-redirects to questdb.com; this commit removes the redirect hop and aligns the source with the rest of the crate's outbound link convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two GitHub tree/blob URLs in the embedded ingress module docs were hard-coded at 6.1.0 but not listed in .bumpversion.toml, so the next release would leave them stale. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The narrow `sync-sender-tcp` and `sync-sender-http` legs verify that the library builds when a downstream consumer enables only one ILP transport. They don't need to compile the crate-level QWP/WebSocket doctests, which assume `sync-sender-qwp-ws` is enabled (the default, also used by docs.rs). Doctest coverage stays in the four wider legs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jerrinot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
questdb-rs/examples/qwp_ws_failover.rs (1)
35-38: ⚡ Quick winAvoid Unix-only
sf_dirin example config.Hardcoding
/tmp/...makes the example non-portable. Prefer deriving a temp path at runtime (or passing it via CLI/env) so the sample works cross-platform.♻️ Proposed cross-platform tweak
- let conf = "ws::addr=db-primary:9000,db-replica-1:9000,db-replica-2:9000;\ - sf_dir=/tmp/myapp-qdb-sf;\ - sender_id=ingest-1;\ - reconnect_max_duration_millis=300000;"; + let sf_dir = std::env::temp_dir().join("myapp-qdb-sf"); + let conf = format!( + "ws::addr=db-primary:9000,db-replica-1:9000,db-replica-2:9000;\ + sf_dir={};\ + sender_id=ingest-1;\ + reconnect_max_duration_millis=300000;", + sf_dir.display() + ); - let mut sender = Sender::from_conf(conf)?; + let mut sender = Sender::from_conf(conf.as_str())?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@questdb-rs/examples/qwp_ws_failover.rs` around lines 35 - 38, The example hardcodes a Unix-only sf_dir in the conf string (variable conf); change it to derive a cross-platform temporary directory at runtime or accept it from an env/CLI value and interpolate that path into conf instead of "/tmp/...". Locate the conf binding in qwp_ws_failover.rs, create or use a temp_dir (e.g., std::env::temp_dir() or an env var) and replace the hardcoded sf_dir value in the conf string with that runtime-derived path so the example works on Windows and other platforms.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@questdb-rs/README.md`:
- Around line 155-158: Update the mailing list/community link text in the README
(the paragraph containing "join us on Slack" and "mailing list") to use the
canonical questdb.com domain instead of questdb.io; locate the sentence with the
"[mailing list](https://questdb.io/community/)" link and replace the URL with
"https://questdb.com/community/" so the README consistently uses questdb.com.
---
Nitpick comments:
In `@questdb-rs/examples/qwp_ws_failover.rs`:
- Around line 35-38: The example hardcodes a Unix-only sf_dir in the conf string
(variable conf); change it to derive a cross-platform temporary directory at
runtime or accept it from an env/CLI value and interpolate that path into conf
instead of "/tmp/...". Locate the conf binding in qwp_ws_failover.rs, create or
use a temp_dir (e.g., std::env::temp_dir() or an env var) and replace the
hardcoded sf_dir value in the conf string with that runtime-derived path so the
example works on Windows and other platforms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3a31474-d55c-414c-87f9-62e53f3c2383
📒 Files selected for processing (13)
.bumpversion.tomlci/run_all_tests.pyquestdb-rs/Cargo.tomlquestdb-rs/README.mdquestdb-rs/examples/qwp_ws_basic.rsquestdb-rs/examples/qwp_ws_error_handling.rsquestdb-rs/examples/qwp_ws_failover.rsquestdb-rs/src/ingress.rsquestdb-rs/src/ingress/buffer.rsquestdb-rs/src/ingress/buffer/ilp.rsquestdb-rs/src/ingress/mod.mdquestdb-rs/src/ingress/sender.rsquestdb-rs/src/ingress/tests.rs
Three hetzner-incus bash steps used `set -eux` and then emitted `##vso[task.setvariable variable=JAVA_HOME...]` lines. Under xtrace, bash also writes the command to stderr with single-quoted arguments (`+ echo '##vso[...]/usr/lib/jvm/java-25-openjdk-amd64'`). The Azure agent parses `##vso` from both stdout and stderr and "last line wins", so the stderr copy intermittently leaks a trailing `'` into JAVA_HOME, making the next Maven@3 step fail with Not found jdkUserInputPath: /usr/lib/jvm/java-25-openjdk-amd64' Disable xtrace immediately before the echoes. The blog post linked in the comment describes the same race and the same fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the questdb.io -> questdb.com migration applied to ilp.rs in 0272c76. The slack.questdb.io invite shortener and the homepage link on line 3 are intentionally left alone -- the former has no .com equivalent, the latter is out of scope here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@questdb-rs/examples/qwp_ws_error_handling.rs`:
- Around line 67-71: The early return on the wait_error path skips graceful
shutdown; ensure sender.close_drain() is always invoked before returning the
error. Change the error branch that currently reads if let Some(err) =
wait_error { return Err(err); } to first call sender.close_drain()? (or
otherwise ensure close_drain runs, e.g., via a scope guard) and then return
Err(err), so sender.close_drain() runs on both success and error paths;
reference the wait_error variable and the sender.close_drain() call when making
this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70199c77-69f9-489c-80ca-5c441b4a0923
📒 Files selected for processing (4)
questdb-rs/examples/qwp_ws_basic.rsquestdb-rs/examples/qwp_ws_error_handling.rsquestdb-rs/examples/qwp_ws_failover.rsquestdb-rs/src/ingress/mod.md
Promote the effective initial-connect retry mode to sync when a QWP/WebSocket reconnect policy knob is explicitly configured, unless the user explicitly set initial_connect_retry. Document the implicit promotion rule and add parser/runtime coverage for the promotion and explicit opt-out behavior.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Reorient the crate's user-facing documentation around QWP/WebSocket (the recommended default transport) instead of leading with ILP. ILP/TCP and ILP/HTTP remain documented under a "Legacy" section.
Changes
Docs (bulk of the diff)
src/ingress/mod.md(crate guide) rewritten: QWP/WebSocket as the primary surface, with sections for FSN-based completion, failover, store-and-forward, durable ACKs, auth, TLS, error handling, and shutdown.README.mdrestructured to match; inline "Protocol Versions" section removed (it belonged with ILP).Buffer/Sender/SenderBuilder/ingressrustdoc trimmed; deep coverage links out to the questdb.com Rust guide. Buffer dispatch comments updated to reflect the QWP/WebSocket path.questdb.ioand private-module URLs migrated to canonicalquestdb.com;bumpversiontracks the versioned URLs inmod.md.Examples —
examples/qwp_ws_basic.rs,qwp_ws_failover.rs,qwp_ws_error_handling.rs.Small fix —
Sender::from_confnow rejectsauto_flush_interval(no transport supports it; previously accepted silently).CI — The single-transport matrix legs (
sync-sender-tcponly,sync-sender-httponly) now run--lib --tests --examples, skipping doctests. The crate-level docs target the default surface where QWP/WS is enabled, matching what docs.rs builds. Doctests still run in the four wider legs.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores