Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

*: introduce an option UseLeaseTTL for lease manager's TTL #1692

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dongsupark
Copy link
Contributor

  • Introduce an option UseLeaseTTL. Default is false.
  • If UseLeaseTTL turned on, create lease manager with the TTL value.
  • When gRPC turned on, remove previous workaround of setting leaseTTL
    to a huge value.
  • Increase engine reconcile interval from 2 to 5 sec.

Originally written by @hectorj2f
Taken from https://github.com/giantswarm/fleet/tree/patch_lease_ttl

* Introduce an option UseLeaseTTL. Default is false.
* If UseLeaseTTL turned on, create lease manager with the TTL value.
* When gRPC turned on, remove previous workaround of setting leaseTTL
  to a huge value.
* Increase engine reconcile interval from 2 to 5 sec.

Originally written by Hector Fernandez <hectorj@gmail.com>
Taken from https://github.com/giantswarm/fleet/tree/patch_lease_ttl
@dongsupark
Copy link
Contributor Author

dongsupark commented Dec 9, 2016

It turns out that this PR results in performance regressions. Nomi benchmark shows a large number of missing units, as well as a huge variation of unit start times.
Its reason is that leaseTTL is not set to a big value any more in Engine.Run(), in case of enable_grpc==true.
If the leaseTTL is set to a big value again, performance becomes normal again. Though in that case, there's no point in introducing the UseLeaseTTL option at all.

Until we could find out a solution, let's not merge.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants