Add locket client keepalive time and timeout to jobs #675
Conversation
|
Correlates to cloudfoundry/locket#12 |
| diego.auctioneer.locket.client_keepalive_time: | ||
| description: "Period in seconds after which the locket gRPC client sends keepalive ping requests to the locket server it is connected to." | ||
| default: 10 | ||
| diego.auctioneer.locket.client_keepalive_timeout: | ||
| description: "Timeout in seconds to receive a response to the keepalive ping. If a response is not received within this time, the locket client will reconnect to another server." | ||
| default: 20 | ||
|
|
There was a problem hiding this comment.
This means it sends a keepalive every 10 seconds right? And that after 20 seconds it times out?
If that is the case, can you change the timeout to be slightly more than 2 times larger than the keep alive? That way if the first keep alive is missed, the second one has a chance to get there in time. Maybe 22 seconds?
Can you also add a template check (and test) for making sure the timeout is greater than the keepalive time?
There was a problem hiding this comment.
Yes, this makes sense. Im not sure where are the job templates being tested. In the bosh docs: https://bosh.io/docs/job-templates/ it is described that we need a bosh-template ruby gem. Currently we do not have something like this, should I create one for every job that we need to test ?
|
FYI, CI seems unhappy because our github-pr resource is confused about the remote branch also being |
|
Thanks @geofffranks, should I just move the changes to a new branch and make a new PR ? It wouldn't take much time. |
|
yeah, that might be easiest in the short term @klapkov. |
|
you might be able to edit this PR to pull from the new branch, so we can keep the existing discussion history. |
As discussed in this issue #627, with this PR we add a keepalive timeout to the locket client, which will improve its performance. The "Time" property of the gRPC client is also added to the locket client config, so operators can configure both of them depending on their needs. Here is the PR in the locket repo: cloudfoundry/locket#12