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

dynamic-lambda: continue tracking the best proposal even after freezing #5701

Merged
merged 275 commits into from
Oct 13, 2023

Conversation

yossigi
Copy link
Contributor

@yossigi yossigi commented Aug 22, 2023

Summary

This PR builds on #5654 and changes the credential propagation and tracking strategy, so we continue propagating credentials for some time after the round concluded and continue to track when we received the lowest credential. This makes the measurements for the dynamic lambda feature independent of the actual filter timeout being set.

Test Plan

Extending existing tests to make sure that better period 0 step 0 credentials get propagated even if they're late.
Extending existing tests to make sure that better period 0 step 0 credentials impact the measurements used in setting the filter timeout.

yossigi and others added 30 commits August 10, 2023 16:43
remove commented out code from when we were trying to get receivedAt for proposal-votes
fix comment to say validated instead of received
agreement/player.go Outdated Show resolved Hide resolved
agreement/proposalManager.go Outdated Show resolved Hide resolved
gmalouf
gmalouf previously approved these changes Oct 11, 2023
Copy link
Contributor

@gmalouf gmalouf left a comment

Choose a reason for hiding this comment

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

The updated naming conventions made this noticeably easier to read/follow. There is a codegen failure to address on the build.

agreement/events.go Outdated Show resolved Hide resolved
agreement/events.go Outdated Show resolved Hide resolved
agreement/service.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

LGTM -- it appears the build error is because msgp generation needs to be run again

jannotti
jannotti previously approved these changes Oct 11, 2023
@cce cce dismissed stale reviews from jannotti and gmalouf via 5923d76 October 12, 2023 00:21
@cce cce merged commit 4249027 into master Oct 13, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants