Adjust key buffer sizes in CI based on parallelism #2275
Conversation
tnozicka
added
kind/flake
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tnozicka The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
scylla-operator-bot
bot
added
approved
tnozicka
added
kind/failing-test
|
#2267 landed |
|
/test images |
|
/retest |
There was a problem hiding this comment.
To make this effective, shouldn't we give the operator some warmup time? Otherwise it seems like it may actually make things worse?
So far we only saw the test with a short timeout fail in periodics which actually have bigger buffers (10/30), so empirically it looks like the smaller buffers could have actually helped get the controllers more cpu time not allocated to generating certs?
| CRYPTO_KEY_BUFFER_SIZE_MIN=6 | ||
| export CRYPTO_KEY_BUFFER_SIZE_MIN | ||
| CRYPTO_KEY_BUFFER_SIZE_MAX=10 | ||
| export CRYPTO_KEY_BUFFER_SIZE_MAX | ||
| if [[ -n "${SO_E2E_PARALLELISM-}" ]]; then | ||
| CRYPTO_KEY_BUFFER_SIZE_MIN=$(( "${CRYPTO_KEY_BUFFER_SIZE_MIN}" * "${SO_E2E_PARALLELISM}" )) | ||
| CRYPTO_KEY_BUFFER_SIZE_MAX=$(( "${CRYPTO_KEY_BUFFER_SIZE_MAX}" * "${SO_E2E_PARALLELISM}" )) | ||
| fi |
There was a problem hiding this comment.
this also needs to be done in ci-deploy-release script, especially that we only saw the tests fail in periodics
There was a problem hiding this comment.
that we only saw the tests fail in periodics
this fails on presubmits as well, see the linked issue
There was a problem hiding this comment.
ok, haven't seen that before (still needs to be wired in the other script)
This is orthogonal, even without it there is still time where we wait for cluster rollout, generate 3 certs and wait to have to little of them later.
not for the certs to my knowledge, it just makes the load constant but I don't think there are other compute extensive task to compete with
nope, see the "resolves issue" which is a presubmit #2274 (comment) |
|
@tnozicka: The following test failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
but this might make more mess without the limits / quaranteed qos, let's wait for that |
Description of your changes:
This PR adjust the key buffer sizes based on the parallelism, so the cache has a real chance to be effectively used. This should help with many flake while keeping the cpu allocation. We can adjust the cpu allocation separately - this just makes a better use of what we have.
Which issue is resolved by this Pull Request:
Resolves #2274