-
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
Prewarm eviction variance #4916
Prewarm eviction variance #4916
Conversation
@@ -1195,4 +1199,24 @@ class ContainerPoolObjectTests extends FlatSpec with Matchers with MockFactory { | |||
pool = pool - 'first | |||
ContainerPool.remove(pool, MemoryLimit.STD_MEMORY) shouldBe List('second) | |||
} | |||
|
|||
it should "remove expired in order of expiration" in { |
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.
Can we add one more test case to make sure no more than prewarmExpirationLimit
containers are removed?
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.
This change looks good to me with one minor nit.
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.
LGTM
… oldest is always removed first
When enabling reactive prewarm configs, if batches of prewarms are removed at the same time across invokers, we end up with a herd of container deletion and creation at the same time, especially if invokers startup at the same time. This can cause scheduling challenges particularly in mesos or kubernetes. Given that these prewarm containers are not in use, we should try to alleviate any extra issues the prewarm state changes may cause, and allow them to linger slightly longer, if there is a benefit to the scheduling system.
Description
Related issue and scope
My changes affect the following components
Types of changes
Checklist:
This PR adds some configs and sensible defaults to allow the reactive prewarm config affects to be varied across invokers to smooth out deletion and rescheduling of prewarm containers: