Skip to content

fix(configs): reject clusters exceeding VSR replica limit - #4661

Open
Radiancebobo wants to merge 2 commits into
apache:masterfrom
Radiancebobo:fix/vsr-replica-count-validation
Open

Radiancebobo wants to merge 2 commits into
apache:masterfrom
Radiancebobo:fix/vsr-replica-count-validation

Conversation

@Radiancebobo

Copy link
Copy Markdown
Contributor

Which issue does this PR address?

Closes #4659

Rationale

VSR uses fixed-capacity structures for at most 32 replicas, but cluster configuration validation previously accepted larger rosters. A cluster with more than 32 nodes could therefore pass startup validation and later panic when a replica outside the supported range participates in a view change. Rejecting the configuration at startup makes the supported limit explicit and prevents the invalid topology from reaching consensus runtime code.

What changed?

Configuration validation now rejects enabled cluster rosters containing more than 32 nodes and prints an error that identifies the configured count and supported maximum. The configuration-layer limit is pinned to consensus::REPLICAS_MAX with a compile-time assertion, and tests cover acceptance at 32 nodes and rejection at 33.

Local Execution

  • Passed: both boundary tests (validate_accepts_the_vsr_replica_limit and validate_rejects_rosters_above_the_vsr_replica_limit); the configs test suite; cargo fmt --all.

@Radiancebobo

Copy link
Copy Markdown
Contributor Author

/skill team-review-slim

@hubcio

hubcio commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

/skill team-review

@github-actions

Copy link
Copy Markdown

Thanks for the PR. It is labeled S-waiting-on-review and queued for review.

If AI was used to create this PR, you must run the team-review skill before opening it.

Slash commands (own line, regular comment) move it around the queue:

  • /ready - back to S-waiting-on-review after addressing feedback
  • /author - flip to S-waiting-on-author while you finish changes
  • /request-review @user-or-team - request a reviewer
  • /pin - exempt the PR from the stale bot, /unpin to undo

See CONTRIBUTING.md for details.

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Oct 27, 2026
@github-actions

Copy link
Copy Markdown

/skill is limited to repository committers, and Radiancebobo does not have write access.

@github-actions github-actions Bot 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.

Summary: ClusterConfig::validate now rejects rosters above the VSR limit of 32, and a compile-time assert in core/server pins the new constant to consensus::REPLICAS_MAX. The two findings are a u8 conversion that the cap makes unreachable and an operator-facing documentation gap for the new limit.

Counts: critical 0, warning 0, nit 1, simplification 1


This review was generated by Claude Code 2.1.310 on deepseek-flash[1m]. Review the output before you act on it.

@Radiancebobo

Copy link
Copy Markdown
Contributor Author

/request-review @hubcio

@github-actions
github-actions Bot requested a review from hubcio October 27, 2026 12:40
@codecov

codecov Bot commented Oct 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.56%. Comparing base (6e9ca2e) to head (922a9a4).
⚠️ Report is 2 commits behind head on master.

❌ Your patch check has failed because the patch coverage (40.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4661      +/-   ##
============================================
- Coverage     80.89%   80.56%   -0.34%     
+ Complexity     3107     3106       -1     
============================================
  Files          1465     1465              
  Lines        356197   356207      +10     
  Branches     300421   300434      +13     
============================================
- Hits         288158   286990    -1168     
- Misses        61207    62404    +1197     
+ Partials       6832     6813      -19     
Components Coverage Δ
Rust Core 79.76% <100.00%> (-0.40%) ⬇️
Java SDK 78.15% <ø> (ø)
C# SDK 83.58% <ø> (-0.12%) ⬇️
Python SDK 85.67% <ø> (ø)
PHP SDK 90.71% <ø> (+0.04%) ⬆️
Node SDK 96.44% <ø> (ø)
Go SDK 70.05% <ø> (+0.21%) ⬆️
C++ SDK 85.51% <ø> (ø)
Files with missing lines Coverage Δ
core/configs/src/server_config/cluster.rs 84.93% <100.00%> (+0.79%) ⬆️
core/server/src/boot/recovery.rs 82.84% <ø> (-1.84%) ⬇️

... and 563 files with indirect coverage changes

🚀 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.

This branch has not been deployed

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

Labels

S-waiting-on-review PR is waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

config: reject cluster configurations above the VSR replica limit

2 participants