-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Delay after bloating test image #6014
Conversation
Some systems don't prevent memory allocation, but kill any pods that exceed memory usage after some time. So add a 1-second delay after bloating memory in the autoscale test image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coryrc: 0 warnings.
In response to this:
Some systems don't prevent memory allocation, but kill any pods that exceed
memory usage after some time. So add a 1-second delay after bloating memory
in the autoscale test image.Fixes #6007
It's done at this location because it still logs the increase in memory if possible.
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.
The following jobs failed:
Failed non-flaky tests preventing automatic retry of pull-knative-serving-unit-tests:
|
/retest |
/assign @vagababov |
@@ -173,6 +173,7 @@ func handler(w http.ResponseWriter, r *http.Request) { | |||
go func() { | |||
defer wg.Done() | |||
fmt.Fprint(w, bloat(mb)) | |||
time.Sleep(time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we already have a delay of 1s or more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean? The sleep option? Each bloat/sleep/prime etc runs in parallel, so it won't have any unanticipated effects unless a bloat and a sleep < 1s occur in the same call and the test expects it to get back right away (which does not appear to be the case because everything passes)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess. Do you think 1s is enough for things to be killed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is for fully-managed Cloud Run. It has no effect on k8s-based platforms.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: coryrc, vagababov 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 |
/assign @dgerd |
I really don't like the idea of inserting a random sleep to get this to work. In fact I don't like the timing aspect of the test at all, and that the only way to observe failures without timing is to reach down into the Pod. I see two options:
I don't think it is worth the effort to go down the second path at this time. |
Some systems don't prevent memory allocation, but kill any pods that exceed
memory usage after some time. So add a 1-second delay after bloating memory
in the autoscale test image.
Fixes #6007
It's done at this location because it still logs the increase in memory if possible.