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

Reduce the wait time of the update monitoring loop #667

Closed
brendandixon opened this issue Apr 12, 2017 · 2 comments
Closed

Reduce the wait time of the update monitoring loop #667

brendandixon opened this issue Apr 12, 2017 · 2 comments
Milestone

Comments

@brendandixon
Copy link
Contributor

When the daemon launches the child (GoalState processing) agent, it loops for 15 minutes, checking the status of the child process once a minute, to ensure the child successfully launched and processed.

The child process, before processing GoalState, first looks for updates. It immediately exits is an update is discovered. This process is generally quick, less then a few seconds.

Unfortunately, the Popen Python object (in 2.7, at least, which we must support) used to launch the child does not support waiting against the process with a timeout. The daemon emulates waiting with a timeout by checking status and then sleeping. This means the daemon will not discover and launch the downloaded update for roughly a minute.

We should reduce the sleep to something much smaller, say 3-5 seconds, or perhaps even 1 second since this occurs early in VM launch and CPU consumption is not the main issue.

@hglkrijger hglkrijger added this to the v2.2.9 milestone Apr 12, 2017
@brendandixon
Copy link
Contributor Author

@hglkrijger Let's poll at 1 second intervals for the first two minutes and then back off after that...or just poll at 1 second. My only concern is a high polling rate while the child is doing real work.

Alternatively, we could poll at a high frequency only if there are no downloaded agents (that is, an update is likely and this is likely a new VM). That would work quite well.

Your thoughts?

hglkrijger pushed a commit that referenced this issue Apr 17, 2017
Signed-off-by: Brendan Dixon <brendandixon@me.com>
@hglkrijger
Copy link
Member

fix merged

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

No branches or pull requests

2 participants