Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

updater: Decrease default monitoring period #1967

Merged
merged 1 commit into from
Feb 24, 2017

Conversation

aaronlehmann
Copy link
Collaborator

The updater monitors a tasks after they start to make sure they don't
fail immediately. The default time interval for this is 30s. However,
this is a bit long for synchronous updates to wait. Reduce it to 5s.
Users who would like a longer monitoring period should specify their own
value, or ideally use a health check in the service definition.

The updater monitors a tasks after they start to make sure they don't
fail immediately. The default time interval for this is 30s. However,
this is a bit long for synchronous updates to wait. Reduce it to 5s.
Users who would like a longer monitoring period should specify their own
value, or ideally use a health check in the service definition.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@codecov-io
Copy link

Codecov Report

Merging #1967 into master will increase coverage by 0.9%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #1967     +/-   ##
=========================================
+ Coverage   53.57%   54.48%   +0.9%     
=========================================
  Files         109      108      -1     
  Lines       18897    18587    -310     
=========================================
+ Hits        10125    10128      +3     
+ Misses       7541     7235    -306     
+ Partials     1231     1224      -7

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea0cd1e...bc74a25. Read the comment docs.

@dongluochen
Copy link
Contributor

LGTM

@dperny
Copy link
Collaborator

dperny commented Feb 21, 2017

Looks fine to me.

@dperny
Copy link
Collaborator

dperny commented Feb 21, 2017

Should this impact performance at all, actually?

@aaronlehmann
Copy link
Collaborator Author

It shouldn't really impact performance. The main user-visible impact is that "update state" will switch to "complete" 5 seconds after updating the last task (if all goes well), instead of 30 seconds after. Right now this is a relatively mundane detail, but the WIP PR I have open in docker that shows progress bars for updates makes it a much bigger deal.

@aaronlehmann aaronlehmann merged commit 7b7b440 into moby:master Feb 24, 2017
@aaronlehmann aaronlehmann deleted the default-monitoring-period branch February 24, 2017 01:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants