ETCD-403: Revert user configuration for etcd and change default max quota#1506
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@dusk125: This pull request references ETCD-403 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/assign @dhellmann @pmtk |
|
/hold We want to see the memory behavior of etcd during a defragment of a large (near 8gb quota) database to ensure that it does not grow in memory usage over time and doesn't heavily spike in usage in the short term. We should expect to see a sawtooth pattern of usage. |
openshift-ci
bot
added
the
do-not-merge/hold
|
We are pretty confident that we do want this change, so I'll approve the PR. We should wait for the measurements before merging it, as Allen says. /approve |
openshift-ci
bot
added
the
approved
dusk125
force-pushed
the
revert-etcd-config
branch
from
03daad1 to
31cdc11
Compare
|
So it looks like etcd has a memory overhead of 30-60% (during times of heavy use) in addition to attempting to hold the database file(s) in memory. Doug and I agreed that keeping the quota at 8GB and allowing the user to opt-in to a memory limit was a good way forward. The most recent version of this PR introduces a memory limit for etcd that a user can add if they want; by default, there's no limit. I ran a synthetic 'put' test against a fresh etcd server with varying memory limits and found that 50MB is a good minimum before etcd performance degrades significantly. Therefore, Microshift config will fail to parse if a user attempts to set a value lower than 50MB. |
dhellmann
left a comment
dhellmann
left a comment
There was a problem hiding this comment.
Could you update docs/howto_config.md as part of this PR, too?
dusk125
force-pushed
the
revert-etcd-config
branch
from
31cdc11 to
e74e7a3
Compare
|
I had one nit, and there's a hold. We can address the nit separately, so if we're ready to move ahead otherwise please remove the hold. |
|
/retest |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
dusk125
force-pushed
the
revert-etcd-config
branch
from
e74e7a3 to
dfd0253
Compare
dusk125
force-pushed
the
revert-etcd-config
branch
from
dfd0253 to
8d231d7
Compare
dhellmann
left a comment
dhellmann
left a comment
There was a problem hiding this comment.
2 doc nits and then I think this is ready to merge
dusk125
force-pushed
the
revert-etcd-config
branch
from
8d231d7 to
042457a
Compare
|
/retest |
|
/lgtm |
|
@dhellmann: once the present PR merges, I will cherry-pick it on top of release-4.13 in a new PR and assign it to you. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/test e2e-openshift-conformance-reduced |
|
This version looks good. Let's see if these are just test flakes. |
|
/test e2e-openshift-conformance-reduced |
|
I was able to see that etcd is running in the sos report in the CI failures, so I think these are just flakes as well. |
|
/retest |
dusk125
force-pushed
the
revert-etcd-config
branch
from
fb9f2dd to
e836402
Compare
|
/test e2e-greenboot-arm |
dusk125
force-pushed
the
revert-etcd-config
branch
2 times, most recently
from
9f26584 to
c664334
Compare
|
I think this strikes the right balance. Let's see what CI has to say. /lgtm |
|
/test e2e-router-smoke-test |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/retest-required Remaining retests: 0 against base HEAD ea60015 and 2 for PR HEAD c664334184d559b68fac9b7dd69d509dece55341 in total |
|
/retest |
|
/retest-required Remaining retests: 0 against base HEAD a94f74f and 1 for PR HEAD c664334184d559b68fac9b7dd69d509dece55341 in total |
dusk125
force-pushed
the
revert-etcd-config
branch
from
c664334 to
c2f45a4
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhellmann, dusk125 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@dusk125: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@dhellmann: new pull request created: #1604 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
7 participants
To simplify the configuration, this PR reverts the changes made by #1435. Also, this sets the maximum on-disk quota for etcd to be the maximum allowed (and aligned with Openshift) as the default to reduce the likelihood that a customer will hit the quota during runtime.