Skip to content

Conversation

@timvaillancourt
Copy link
Contributor

@timvaillancourt timvaillancourt commented Dec 17, 2025

Description

This PR adds to VTOrc the knowledge of what Vitess Cell a VTOrc process is running within. This knowledge will be used to gain confidence in certain VTOrc problems, by asking another cell to validate the detected problem

Today some problems (example vttablet down on PRIMARY) are not actioned by VTOrc, because it does not have enough confidence in it's detection from a single angle. On the opposite extreme, sometimes VTOrc believes it is correct using limited details, from only it's perspective

The topology recovery side of this change will come in future PRs, for now having this in v24 will be useful

Details:

  • --cell flag added to VTOrc
    • This flag will become required (like vtgate) in v25+
    • If defined, the cell is validated using the topo
      • Currently the new-flag value is not used in any VTOrc logic. The goal here was to just introduce the flag
    • If undefined, an error warning this flag will be required in v25+ is logged
  • 100+ end-to-end tests updated for new .StartVTOrc/.StartVTOrcs/etc signature (😞)
  • examples/ shell scripts updated for new flag

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported to release branches
  • If this change is to be back-ported to previous releases, a justification is included in the PR description
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on CI?
  • Documentation was added or is not required

Deployment Notes

AI Disclosure

Signed-off-by: Tim Vaillancourt <[email protected]>
@timvaillancourt timvaillancourt self-assigned this Dec 17, 2025
@timvaillancourt timvaillancourt added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: VTorc Vitess Orchestrator integration labels Dec 17, 2025
@github-actions github-actions bot added this to the v24.0.0 milestone Dec 17, 2025
@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

❌ Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.90%. Comparing base (749fe54) to head (dfb84d1).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
go/vt/vtorc/server/discovery.go 0.00% 15 Missing ⚠️
go/vt/vtorc/config/config.go 0.00% 4 Missing ⚠️
go/cmd/vtorc/cli/cli.go 0.00% 3 Missing ⚠️
go/cmd/vtgate/cli/cli.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #19047      +/-   ##
==========================================
+ Coverage   69.88%   69.90%   +0.02%     
==========================================
  Files        1610     1612       +2     
  Lines      215431   215815     +384     
==========================================
+ Hits       150551   150863     +312     
- Misses      64880    64952      +72     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
skippedRecoveriesMap := vars["SkippedRecoveries"].(map[string]interface{})
skippedCount := GetIntFromValue(skippedRecoveriesMap[mapKey])
assert.EqualValues(c, countExpected, skippedCount)
assert.GreaterOrEqual(c, skippedCount, countExpected)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix for flakiness. 1 or more is fine for this test - the important part is the ERS gets skipped

@promptless
Copy link
Contributor

promptless bot commented Dec 20, 2025

📝 Documentation updates detected!

New suggestion: Document VTOrc --cell flag

--buffer-window duration Duration for how long a request should be buffered at most. (default 10s)
--catch-sigpipe catch and ignore SIGPIPE on stdout and stderr if specified
--cell string cell to use
--cell string cell to use (required)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated change to make it clear to users that VTGate requires this flag

@promptless
Copy link
Contributor

promptless bot commented Dec 20, 2025

📝 Documentation updates detected!

New suggestion: Add changelog entry for VTOrc --cell flag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: VTorc Vitess Orchestrator integration Type: Enhancement Logical improvement (somewhere between a bug and feature)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant