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

feat(f3): remove lease-check interval constants #12580

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

Stebalien
Copy link
Member

@Stebalien Stebalien commented Oct 10, 2024

Related Issues

I expect this may help with some of the flaky tests as those may have been timing out because we weren't renewing the lease fast enough.

#12519

Proposed Changes

  1. Instead of hard-coding the renewal to be within 2 instances of the end of the lease, specify the renewal to be within 1/2th of the end of the lease, rounded up (towards the end of the lease).
  2. Instead of hard-coding the lease check interval to be 10s (problematic for tests where the block time is 100ms), set it to be the minimum amount of time we expect to have to wait for the remaining
    instances to complete.

I've configured a hard-coded minimum of renewing the lease once every 100ms in case a manifest doesn't have a configured catch-up alignment interval, but I don't expect us to hit that in practice.

Checklist

Before you mark the PR ready for review, please make sure that:

@Stebalien Stebalien added the skip/changelog This change does not require CHANGELOG.md update label Oct 10, 2024
@Stebalien Stebalien requested a review from masih October 10, 2024 22:52
1. Instead of hard-coding the renewal to be within 2 instances of the
end of the lease, specify the renewal to be within 1/2th of the end of
the lease, rounded up (towards the end of the lease).
2. Instead of hard-coding the lease check interval to be
10s (problematic for tests where the block time is 100ms), set it to be
the minimum amount of time we expect to have to wait for the remaining
instances to complete.
@Stebalien
Copy link
Member Author

Note: it would be nice to tune the backoff in the same way, but that's tricker.

@Stebalien Stebalien merged commit 8ea8e63 into master Oct 11, 2024
82 of 83 checks passed
@Stebalien Stebalien deleted the steb/lease-check-interval branch October 11, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does not require CHANGELOG.md update
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

3 participants