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

Update bootc-fetch-apply-updates.timer with comments for RandomizedDe… #295

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

mrguitar
Copy link
Contributor

@mrguitar mrguitar commented Feb 1, 2024

…laySec=

RandomizedDelaySec= is the best kept secret of timers. Given that such few people know about it and it's very useful for large deployments, I think it makes sense to add it as a comment.

Thoughts about providing this out of the box as a comment? I'm not proposing we turn it on by default.

Copy link

openshift-ci bot commented Feb 1, 2024

Hi @mrguitar. Thanks for your PR.

I'm waiting for a containers member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jeckersb
Copy link
Contributor

jeckersb commented Feb 1, 2024

/ok-to-test

👍 from me, reminds me of messing with cfengine splaytime many moons ago!

Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

I'm in favor of turning it on by default actually...there's not really a strong downside right?

@mrguitar
Copy link
Contributor Author

mrguitar commented Feb 1, 2024

@jeckersb exactly! I always describe this as "splay"!

@cgwalters, I can't think of a downside. Maybe the comment should change to this if we enable it: "When deploying a large number of systems, it may be beneficial to increase this value to help with load on the registry."

I think 2h is a sane default, but I just put that as an example. Should it change?

@mrguitar
Copy link
Contributor Author

mrguitar commented Feb 1, 2024

ok, I tried to make that change, but since I suck at git it's stuck on DCO. :) This is what PM help looks like. lol

@cgwalters
Copy link
Collaborator

See https://docs.pi-hole.net/guides/github/how-to-signoff/

TL;DR do git commit --amend --signoff

RandomizedDelaySec= is the best kept secret of timers.

Signed-off-by: Ben Breard <bbreard@redhat.com>
Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters
Copy link
Collaborator

I force pushed an update to this:

  • squashed the commits
  • Tweaked the commit message now that it's not a comment

@cgwalters cgwalters merged commit eb0d270 into containers:main Feb 2, 2024
11 checks passed
@mrguitar mrguitar deleted the patch-1 branch February 2, 2024 21:01
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.

None yet

3 participants