(PUP-12061) Allow splay and splaylimit to be modified in puppet.conf #9484
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joshcooper
force-pushed
the
splaylimit_daemon
branch
from
5e53f75
to
7b420be
Compare
joshcooper
commented
Sep 26, 2024
…/main/splay" This reverts commit a1a84cb.
Memoize the jobs so it's clearer which jobs 0 and 1 are.
joshcooper
force-pushed
the
splaylimit_daemon
branch
from
7b420be
to
f1c2fb0
Compare
joshcooper
commented
Sep 27, 2024
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
If puppet.conf was modified, but splaylimit wasn't, then we recalculated the splay offset. This goes against the desired behavior of only splaying when the agent is initially started and running every runinterval seconds from that point forward or until runinterval/splaylimit are changed in the future. Now the SplayJob remembers its last splaylimit so that it only recalculates splay if the limit changes. It's important to recalculate splay so that the agent chooses a slot over the new range, either smaller or bigger than before.
Provide more context when the agent runs so it's clearer whether splay is
enabled and what the limits are. We don't want to do this for the reparse and
signal jobs, because they run every 15 and 5 seconds, respectively.
The info message is of the form:
Running agent every 20 seconds with splay 5 of 10 seconds
This message is logged whenever the agent is running periodically, not for
onetime runs. Whether we're daemonized doesn't affect the behavior. This is
because we always call Puppet::Daemon#start when running periodically to
run the event loop.
Previously, enabling or disabling splay in puppet.conf did not work if the agent was already running periodically. If the agent was started with splay disabled, then enabling it would cause a NoMethodError, when trying to call Puppet::Scheduler::Job#splay_limit If the agent was started with splay enabled, then disabling it would have no effect, since we never recalculated the splay limit. To handle these cases, always create a SplayJob for the agent_run job and set its splay_limit to either the limit or 0, depending on whether splay is enabled or not. Setting a splay_limit to 0 causes the splay to also be set to 0 because rand(1) always returns 0.
Since splaylimit defaults to runinterval, changes to runinterval affect splay. Add tests covering cases where runinterval is increased and decreased. It's important to recalculate splay to reduce thundering herds. For example, if runinterval is changed from 30 mins to 60 mins, we want the compute a new slot in the 60 min interval to decrease server load.
joshcooper
force-pushed
the
splaylimit_daemon
branch
from
f1c2fb0
to
15bc22b
Compare
|
Jenkins, please test this |
|
jenkins please test this |
mhashizume
approved these changes
Sep 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If an agent was already running periodically and
splayorsplaylimitwas modified inpuppet.conf, then the agent didn't recalculate its splay mode (enabled/disabled) or its splay offset. A full service restart was required for those changes to take effect.Now the agent will behave as follows when the agent is running periodically:
splayis changed from disabled to enabled, recalculate splay offset based on the currentsplaylimitsplayis changed from enabled to disabled, set splay offset to 0.splaylimitis modified, then recalculate splay based on the newsplaylimitrunintervalis modified andsplaylimitinherits fromruninterval, then recalculate splay offset based on the newrunintervalrunintervalis modified, butsplaylimitdoes not inherit, then keep splay offset as-is.The agent also now logs an info message about splay at information level, see PUP-10732
This behavior change affects the puppet agent service on all platforms except windows because of the way our windows service runs puppet agent --onetime, see PUP-5823
Confusingly, the changes to the
Puppet::Daemonclass affect all periodic runs, even when using--no-daemonize.Although it's possible to splay when running
puppet agent --onetime --no-daemonize --splay, this PR doesn't change how the agent responds to puppet.conf updates. The splay settings are calculated at the beginning of the onetime run and are immutable for the duration. The next timepuppet agent --onetimecommand is run, it will calculate a new splay.