Improve Cosmos perf diagnostics#4797
Open
tvaron3 wants to merge 17 commits into
Open
Conversation
Add an open-loop load-generation mode to the Cosmos perf harness, driven by a fixed arrival rate rather than fixed concurrency. This eliminates coordinated-omission bias: in closed-loop mode a single slow request stalls the next N issuances, hiding tail latency under load. Open-loop mode issues operations on a wall-clock schedule regardless of how long prior operations take. New flags (config.rs): - --target-rate <ops/s>: enables open-loop mode (no default; when unset the harness runs in the existing closed-loop --concurrency mode). - --max-in-flight <N>: caps concurrently outstanding operations in open-loop mode (default 100000). When saturated, excess scheduled issuances are skipped and counted rather than queued, so arrivals never bunch into a catch-up burst. Behavior (runner.rs): - 1ms ticker computes the cumulative target issue count from elapsed wall-clock time; the inner loop issues the per-tick delta. - Each issuance acquires a semaphore permit (try_acquire_owned) before spawning; on saturation it increments a skip counter and logs a WARN every 10000 skips. - Best-effort 30s drain waits for in-flight operations to complete, then prints a total-skipped summary. - Closed-loop path is refactored to share a run_one() helper with the open-loop path; its behavior is unchanged. Known follow-ups (not blocking): - Latency in open-loop mode is measured from operation start inside the spawned task, not from the intended issue time, leaving a small residual omission under executor saturation. - Skip count is reported to stdout only, not persisted to the results document. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove obsolete client-side Gateway V2 configuration now that enablement is service-driven. Bound feed-range query page consumption and add opt-in latency-threshold diagnostics while preserving a zero-overhead default path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Resolve perf harness conflicts while preserving bounded feed-range queries and opt-in latency diagnostics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Record upstream main as the merge parent after resolving the perf harness conflicts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds bounded feed-range queries, open-loop load generation, and opt-in slow-operation diagnostics to the Cosmos performance harness.
Changes:
- Adds configurable page limits and open-loop rate controls.
- Captures and logs diagnostics for successful slow operations.
- Persists the diagnostics threshold in performance results.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/config.rs |
Defines new performance controls. |
src/main.rs |
Validates and wires new configuration. |
src/runner.rs |
Implements scheduling, diagnostics, and reporting. |
src/operations/mod.rs |
Adds diagnostic-bearing operation results. |
src/operations/create_item.rs |
Captures create diagnostics. |
src/operations/read_item.rs |
Captures read diagnostics. |
src/operations/upsert_item.rs |
Captures upsert diagnostics. |
src/operations/query_items.rs |
Collects per-page query diagnostics. |
src/operations/feed_range_query.rs |
Bounds pages and captures diagnostics. |
README.md |
Documents selected new controls. |
Decode the RNTBD backend request duration token and surface it through the standard Cosmos response header so perf runs can record backend latency. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Persist mode-specific open-loop configuration, document its CLI controls, and reject max-in-flight values that exceed Tokio semaphore limits. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Add customer-facing changelog entries for restored backend request duration metadata in Gateway V2 responses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Persist the effective Gateway V2 disabled setting in perf result metadata, including environment overrides and HTTP/2 prerequisite handling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Exercise bounded full-container change feed reads in the Cosmos perf harness and categorize the Gateway V2 duration changelog entries under other changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Surface all response tokens observed in live thin-client traffic through driver and SDK headers. Also finalize bounded change-feed execution and open-loop request/postprocessing accounting from review feedback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Initialize cumulative open-loop issuance and postprocessing counters in the persisted configuration snapshot. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Resolve Cosmos changelog conflicts while preserving PR 4797 entries in the current unreleased sections. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Revert newly added public response-header APIs while retaining Gateway V2 metadata parsing internally. Box large emulator comparison futures so the repository Analyze job passes without dead-code or large-future failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Member
Author
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Add RNTBD protocol terms to the Cosmos spelling dictionary and disable throttle retries in the injected-429 status test so it completes within the live-test timeout. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
tvaron3
marked this pull request as ready for review
July 22, 2026 16:33
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
sdk/cosmos/azure_data_cosmos/CHANGELOG.md:13
- This restores metadata that existing public accessors previously failed to return, so it is a bug fix rather than an "Other Change." Place the entry under the existing
Bugs Fixedheading as required by the changelog categorization guideline.
- Existing `ResponseHeaders` accessors now return Gateway 2.0 backend duration, quota, item-count, and local-LSN response metadata. ([#4797](https://github.com/Azure/azure-sdk-for-rust/pull/4797))
sdk/cosmos/azure_data_cosmos_driver/CHANGELOG.md:13
- This restores response metadata that Gateway 2.0 was dropping, so it belongs under the existing
Bugs Fixedheading rather than "Other Changes." Please use the appropriate changelog category.
- Gateway 2.0 responses now preserve backend duration, quota, item-count, quorum, replica, query, and physical-partition RNTBD metadata when converting to standard Cosmos response headers. ([#4797](https://github.com/Azure/azure-sdk-for-rust/pull/4797))
Distribute bounded change-feed operations round-robin across a force-refreshed physical range cache so every current range receives traffic after splits. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
sdk/cosmos/azure_data_cosmos_driver/CHANGELOG.md:13
- This entry describes restoration of existing response metadata, so it is a bug fix rather than an “Other Change.” Move it under the existing
Bugs Fixedheading, as required by the Cosmos changelog guidance to use the most appropriate category (sdk/cosmos/.github/instructions/cosmos.changelog.instructions.md:12-14).
- Gateway 2.0 responses now preserve backend duration, quota, item-count, quorum, replica, query, and physical-partition RNTBD metadata when converting to standard Cosmos response headers. ([#4797](https://github.com/Azure/azure-sdk-for-rust/pull/4797))
sdk/cosmos/azure_data_cosmos/CHANGELOG.md:13
- This says existing
ResponseHeadersaccessors are being restored for Gateway 2.0, which is a bug fix rather than an “Other Change.” Move it under the existingBugs Fixedheading, following the Cosmos changelog categorization rule (sdk/cosmos/.github/instructions/cosmos.changelog.instructions.md:12-14).
- Existing `ResponseHeaders` accessors now return Gateway 2.0 backend duration, quota, item-count, and local-LSN response metadata. ([#4797](https://github.com/Azure/azure-sdk-for-rust/pull/4797))
Count all remaining overdue arrivals in one update after max-in-flight saturation, avoiding per-arrival scheduler spin at extreme target rates. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 201800be-5c30-4e21-be03-1980fc219c3a
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
sdk/cosmos/azure_data_cosmos/CHANGELOG.md:13
- This restores existing customer-facing
ResponseHeadersbehavior, so it is a bug fix rather than an “Other Change.” Move this entry under the existingBugs Fixedheading so the release notes classify the fix correctly.
- Existing `ResponseHeaders` accessors now return Gateway 2.0 backend duration, quota, item-count, and local-LSN response metadata. ([#4797](https://github.com/Azure/azure-sdk-for-rust/pull/4797))
sdk/cosmos/azure_data_cosmos_driver/CHANGELOG.md:13
- This change restores response metadata that Gateway 2.0 previously dropped, so it belongs under
Bugs Fixed, notOther Changes. Move the entry to the existing bug-fix category to keep the release notes accurate.
- Gateway 2.0 responses now preserve backend duration, quota, item-count, quorum, replica, query, and physical-partition RNTBD metadata when converting to standard Cosmos response headers. ([#4797](https://github.com/Azure/azure-sdk-for-rust/pull/4797))
analogrelay
moved this from In Progress
to Needs Review
in CosmosDB Rust SDK and Driver
NaluTripician
left a comment
NaluTripician
left a comment
Contributor
There was a problem hiding this comment.
Let's meet offline to discuss the changes vs #4789
| ) { | ||
| let diagnostics: Vec<_> = diagnostics | ||
| .iter() | ||
| .map(|context| diagnostics_to_json(context)) |
Contributor
There was a problem hiding this comment.
diagnostics_to_json does DiagnosticsContext::to_json_string() then re-parses with .expect("…always valid JSON"). #4789 rewrites the driver diagnostics_context.rs (+1354) and adds compaction.rs. Question: does #4789 change the JSON shape (breaking downstream dashboards that parse this blob), and does #4789's new impl still guarantee to_json_string is always-valid JSON so that .expect can't panic?
| let mut stream = Box::pin(iterator.take(self.max_pages)); | ||
|
|
||
| let mut backend_total: Option<Duration> = None; | ||
| let mut diagnostics = Vec::new(); |
Contributor
There was a problem hiding this comment.
The harness-level Vec is small (≤ max_pages, default 4). But each element is a DiagnosticsContext that, under a retry storm on a single page, can hold an unbounded per-attempt request list, exactly what #4789's DiagnosticsOptions::max_request_diagnostics is designed to cap. So the harness's slow-op logging (Stop 7) is only bounded once #4789 lands. Question: does #4797 depend on #4789's bound to avoid a log explosion when a slow op is slow because it retried 500 times, or is that context already bounded on main?
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
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.
This PR updates Gateway V2 configuration/result metadata, bounds feed-range queries, adds opt-in slow-success diagnostics, and adds a default-enabled per-feed-range change-feed workload that rotates physical ranges round-robin and consumes at most four pages per operation; use
--no-change-feedwhen comparing with runs created before this change because the default operation mix has changed. It also adds an open-loop target-rate mode.Validated with
cargo build,cargo clippy --all-features --all-targets,cargo doc --no-deps --all-features, andcargo test --all-featuresforazure_data_cosmos_perf, plus live Gateway V2 coverage for restored RNTBD response metadata.