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 <[email protected]>
Taken from https://github.com/giantswarm/fleet/tree/patch_lease_ttl
@dongsupark dongsupark force-pushed the dongsu/use-lease-ttl branch from 9f33b77 to 8f9bb41 Compare December 7, 2016 15:34
@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.

@dongsupark dongsupark force-pushed the master branch 6 times, most recently from 39a99ba to 44591b0 Compare December 15, 2016 19:48
@dongsupark dongsupark added this to the vfuture milestone Dec 16, 2016
@dongsupark dongsupark force-pushed the master branch 3 times, most recently from 0132632 to 6974811 Compare February 8, 2017 10:21
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.

2 participants