test removal of lightweight threads #502
Conversation
z103cb left a comment
z103cb left a comment There was a problem hiding this comment.
You'll need to do something more envolved for the backoff fix.
| if updateNewJob.Status.State != arbv1.AppWrapperStateFailed { | ||
| klog.Infof("[PreemptQueueJobs] Adding preempted AppWrapper %s/%s to back off queue.", aw.Name, aw.Namespace) | ||
| go qjm.backoff(updateNewJob, "PreemptionTriggered", string(message)) | ||
| qjm.backoff(updateNewJob, "PreemptionTriggered", string(message)) |
There was a problem hiding this comment.
@asm582 removing this will block the thread for 20 seconds. There's a sleep in there. Doing this correctly, would require that you setup a a backoff queueu, where the appwrappers will "sleep".
I would not advise for this change.
There was a problem hiding this comment.
back-off does two things add to AddUnschedulableIfNotPresent and later move to MoveToActiveQueueIfExists. if it just does one thing that sleep can be avoided. this may call for implementing separate thread to add AWs to MoveToActiveQueueIfExists
There was a problem hiding this comment.
There's a sleep in there: time.Sleep(time.Duration(qjm.serverOption.BackoffTime) * time.Second). Sleep is not blocking per see as far as this Stack Overflow, but in general not to be used.
There was a problem hiding this comment.
If you want to get rid of the asynchronous backoff call, you definitely need to do something about moving the Appwrapper to a different "sleep" queue and have a routine that moves it from the "sleep" to active.
| qjm.quotaManager.Release(qj) | ||
| } | ||
| go qjm.backoff(qj, dispatchFailedReason, dispatchFailedMessage) | ||
| qjm.backoff(qj, dispatchFailedReason, dispatchFailedMessage) |
| PR needs rebase. 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. |
No description provided.