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

ForceMaster election library #1453

Merged
merged 2 commits into from Feb 13, 2020
Merged

ForceMaster election library #1453

merged 2 commits into from Feb 13, 2020

Conversation

@gdbelvin
Copy link
Collaborator

gdbelvin commented Feb 13, 2020

The previous NoopElection broke the API contract of the election factory
because it did not cancel contexts upon resigning. As a result, the KT
sequencer hung attempting to resign mastership.

This PR introduces a new forcemaster.Election which respects the election
API, and tests it against the election.Tracker.

@gdbelvin gdbelvin requested a review from google/keytransparency as a code owner Feb 13, 2020
@gdbelvin gdbelvin requested a review from AlCutter Feb 13, 2020
@googlebot googlebot added the cla: yes label Feb 13, 2020
@gdbelvin gdbelvin force-pushed the gdbelvin:focemaster branch from a208c5e to ebb1dc8 Feb 13, 2020
@gdbelvin gdbelvin added the bug label Feb 13, 2020
@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 13, 2020

Codecov Report

Merging #1453 into master will increase coverage by 1.36%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1453      +/-   ##
==========================================
+ Coverage   66.59%   67.95%   +1.36%     
==========================================
  Files          54       54              
  Lines        4026     4026              
==========================================
+ Hits         2681     2736      +55     
+ Misses        956      890      -66     
- Partials      389      400      +11     
Impacted Files Coverage Δ
core/sequencer/trillian_client.go 58.57% <0.00%> (-7.15%) ⬇️
core/sequencer/server.go 72.96% <0.00%> (-1.31%) ⬇️
impl/sql/directory/storage.go 69.17% <0.00%> (ø) ⬆️
core/keyserver/revisions.go 64.62% <0.00%> (+2.04%) ⬆️
core/sequencer/election/tracker.go 70.11% <0.00%> (+70.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 128f290...563a5f1. Read the comment docs.

The previous NoopElection broke the API contract of the election
factory because it did not cancel contexts upon resigning.
As a result, the KT sequencer hung attempting to resign mastership.

This PR introduces a new `forcemaster.Election` which respects the
election API, and tests it against the `election.Tracker`.
@gdbelvin gdbelvin force-pushed the gdbelvin:focemaster branch from ebb1dc8 to 563a5f1 Feb 13, 2020
@gdbelvin gdbelvin requested review from Mercurrent and removed request for AlCutter Feb 13, 2020
Copy link

Mercurrent left a comment

Why ForceMaster?

@gdbelvin

This comment has been minimized.

Copy link
Collaborator Author

gdbelvin commented Feb 13, 2020

@gdbelvin gdbelvin merged commit abd6967 into google:master Feb 13, 2020
5 checks passed
5 checks passed
GolangCI No issues found!
Details
Travis CI - Pull Request Build Passed
Details
cla/google All necessary CLAs are signed
codecov/patch 75.00% of diff hit (target 66.59%)
Details
codecov/project 67.95% (+1.36%) compared to 128f290
Details
@gdbelvin gdbelvin deleted the gdbelvin:focemaster branch Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.