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

go/runtime/committee: Close delay and reduced resets #2822

Merged
merged 11 commits into from
Apr 7, 2020

Conversation

kostko
Copy link
Member

@kostko kostko commented Apr 6, 2020

Fixes #2825

  • go/runtime/committee: Introduce close delay when rotating connections

    Previously a connection was immediately closed, interrupting any in-flight
    requests. This introduces a configurable (via WithCloseDelay option) close
    delay so rotated connections are only closed after some time.

  • go/runtime/committee: Don't reset committees when they don't change

    Previously each committee election triggered a reset of all connections for
    that committee. This changes the logic to just bump the committee version in
    case all the committee members are the same.

@kostko kostko force-pushed the kostko/feature/rt-cmte-client-imp branch from d05b14c to 37af819 Compare April 6, 2020 08:49
@codecov
Copy link

codecov bot commented Apr 6, 2020

Codecov Report

Merging #2822 into master will increase coverage by 1.25%.
The diff coverage is 79.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2822      +/-   ##
==========================================
+ Coverage   65.76%   67.01%   +1.25%     
==========================================
  Files         343      342       -1     
  Lines       32938    32968      +30     
==========================================
+ Hits        21663    22095     +432     
+ Misses       8557     8165     -392     
+ Partials     2718     2708      -10     
Impacted Files Coverage Δ
go/worker/sentry/grpc/init.go 25.49% <0.00%> (+0.24%) ⬆️
go/worker/sentry/grpc/worker.go 10.56% <ø> (ø)
go/keymanager/client/client.go 72.11% <66.66%> (-0.11%) ⬇️
go/runtime/committee/nodes.go 80.39% <72.72%> (-0.26%) ⬇️
go/storage/client/client.go 77.09% <76.47%> (+0.32%) ⬆️
go/runtime/committee/client.go 75.86% <78.88%> (-0.33%) ⬇️
go/worker/registration/worker.go 66.73% <88.88%> (+0.82%) ⬆️
go/runtime/committee/committee.go 79.00% <93.33%> (+4.97%) ⬆️
go/common/identity/identity.go 77.60% <100.00%> (+1.32%) ⬆️
go/storage/metrics.go 78.26% <0.00%> (-13.05%) ⬇️
... and 40 more

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 5e9b560...2947898. Read the comment docs.

@kostko kostko marked this pull request as ready for review April 6, 2020 09:12
go/runtime/committee/committee.go Show resolved Hide resolved
@kostko kostko force-pushed the kostko/feature/rt-cmte-client-imp branch from 37af819 to 33a2e7d Compare April 6, 2020 11:09
@kostko kostko force-pushed the kostko/feature/rt-cmte-client-imp branch from 55d0433 to 3af1dec Compare April 6, 2020 12:52
kostko added 7 commits April 7, 2020 11:56
Previously a connection was immediately closed, interrupting any in-flight
requests. This introduces a configurable (via WithCloseDelay option) close delay
so rotated connections are only closed after some time.
Previously each committee election triggered a reset of all connections for that
committee. This changes the logic to just bump the committee version in case all
the committee members are the same.
@kostko kostko force-pushed the kostko/feature/rt-cmte-client-imp branch from f28dbb6 to 08c52fa Compare April 7, 2020 09:57
@kostko kostko force-pushed the kostko/feature/rt-cmte-client-imp branch from 08c52fa to 6bd4dae Compare April 7, 2020 10:00
kostko added 3 commits April 7, 2020 12:08
Since gRPC supports the WithResolvers option to specify local resolvers there is
no need to use the global resolver registry hack.
@kostko kostko force-pushed the kostko/feature/rt-cmte-client-imp branch from 6bd4dae to 39484d4 Compare April 7, 2020 10:09
@kostko kostko merged commit 44847d9 into master Apr 7, 2020
@kostko kostko deleted the kostko/feature/rt-cmte-client-imp branch April 7, 2020 11:28
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.

Panic / crash when validator fails to query sentry for address
4 participants