Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app: Only perform spare check as of a certain cluster size. #139

Conversation

MathieuBordere
Copy link
Contributor

@MathieuBordere MathieuBordere commented Jan 9, 2024

run
https://github.com/canonical/jepsen.dqlite/actions/runs/7455383815/job/20284385625

assertion

 :assert {:valid? false,
          :count 1,
          :matches ({:node "n4",
                     :line "2024/01/09 01:29:31.730561 for jepsen: extra online spare"})},

log

2024/01/09 01:29:31.730561 for jepsen: extra online spare
2024/01/09 01:29:31.730580 changes={{3 3} map[{3297041220608546238 10.2.1.11:8081 voter}:0xc000428f20 {13470429068838499797 10.2.1.15:8081 spare}:0xc000428f10]} offlines=[] byFailureDomain=map[0:[{3297041220608546238 10.2.1.11:8081 voter} {13470429068838499797 10.2.1.15:8081 spare}]]

This looks like a false positive triggered by the following logic in go-dqlite:
https://github.com/canonical/go-dqlite/blob/4edab5e2dd71e25576b5095b2f3ebdea2664e1a0/app/roles.go#L135
The jepsen cluster doesn't have the minimum required size, because the member nemesis has removed a bunch of members.

The fix is to perform a specific check when the cluster doesn't have a certain minimum size.

@cole-miller cole-miller self-requested a review January 9, 2024 14:54
@cole-miller
Copy link
Contributor

cole-miller commented Jan 9, 2024

Thanks, good point! I missed that wrinkle in the role management implementation. Would it be a good idea to tighten the code here to check that if the number of cluster members is less than the target number of voters, we have one voter + N spares?

@MathieuBordere MathieuBordere merged commit 3662b55 into canonical:master Jan 9, 2024
79 checks passed
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