Skip to content

NCBC-4282: Improve WaitUntilReady to follow RFC - #160

Open
emilienbev wants to merge 11 commits into
masterfrom
NCBC-4282
Open

NCBC-4282: Improve WaitUntilReady to follow RFC#160
emilienbev wants to merge 11 commits into
masterfrom
NCBC-4282

Conversation

@emilienbev

Copy link
Copy Markdown
Contributor

No description provided.

@emilienbev
emilienbev marked this pull request as ready for review August 14, 2026 14:56
@davidkelly
davidkelly requested a lite review from Copilot August 17, 2026 23:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds RFC-61 aligned readiness evaluation for WaitUntilReady and modernizes diagnostics pings by using per-node HTTP health endpoints (Query/Analytics/Search), including an exponential backoff to reduce polling load.

Changes:

  • Introduced WaitUntilReadyEvaluator (+ backoff/result types) and comprehensive unit tests for readiness rules.
  • Updated DiagnosticsReportProvider to ping Query/Analytics/Search via node-local HTTP health endpoints and adjusted endpoint scoping for MDS clusters.
  • Refactored cluster/bucket WaitUntilReadyAsync loops to use the shared evaluator, include backoff, and improve logging + argument validation.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/Couchbase.UnitTests/Diagnostics/WaitUntilReadyEvaluatorTests.cs Adds unit coverage for expected-services selection, readiness evaluation, and backoff behavior.
tests/Couchbase.UnitTests/Diagnostics/DiagnosticsReportProviderTests.cs Adds unit coverage for HTTP ping URI building and HTTP ping behavior (status/cancellation).
tests/Couchbase.UnitTests/CouchbaseBucketTests.cs Adds bucket-level tests for invalid desired state and non-positive timeout handling.
src/Couchbase/Diagnostics/WaitUntilReadyEvaluator.cs Implements shared readiness evaluation, expected-services filtering, and exponential backoff.
src/Couchbase/Diagnostics/DiagnosticsReportProvider.cs Switches Query/Analytics/Search pings to node-local HTTP health endpoints and improves service scoping.
src/Couchbase/Core/BucketBase.cs Refactors bucket WaitUntilReadyAsync to use evaluator + backoff and improves validation/logging.
src/Couchbase/Cluster.cs Refactors cluster WaitUntilReadyAsync to use evaluator + backoff and improves validation/logging.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Couchbase/Diagnostics/WaitUntilReadyEvaluator.cs Outdated
Comment thread src/Couchbase/Core/BucketBase.cs Outdated
Comment thread src/Couchbase/Cluster.cs Outdated
Comment thread src/Couchbase/Diagnostics/DiagnosticsReportProvider.cs Outdated
Comment thread src/Couchbase/Diagnostics/WaitUntilReadyEvaluator.cs
- The timeout only applied when no token was supplied, so WaitUntilReady let one unresponsive node stall the whole ping pass.
- A caller waiting for Degraded can now succeed while a node is a black hole.
- Cluster and bucket held the same loop and the same two log messages, they now call one evaluator method and keep only what differs.
- The cluster used a fixed 100 ms delay while waiting for bootstrap, the shared backoff now covers that wait.
An empty expected set means ready, so a bucket with no topology would report success before bootstrap. The bucket now waits for a config or a node before it evaluates a pass.
- Mark the cluster context of ExpectedServices as nullable, it is already read as such.
- Drop a dead term from the Online check.
- Name the case that makes the broad catch around GetNodes deliberate.
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

FIT performer image

Published:

ghcr.io/couchbase/dotnet-fit-performer:NCBC-4282

Run FIT locally against this PR:

fit run preset <preset-name> --performer dotnet-fit-performer:NCBC-4282

Or run it from the workflow here.

Note

Each push to this PR replaces the image and edits this comment. The image is deleted 7 days after the last push.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/Couchbase/Diagnostics/DiagnosticsReportProvider.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants