- 
                Notifications
    
You must be signed in to change notification settings  - Fork 25.6k
 
Don't immediately scale down startups triggered by non-master nodes in inference adaptive allocations. #125297
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
Don't immediately scale down startups triggered by non-master nodes in inference adaptive allocations. #125297
Conversation
…n inference adaptive allocations.
| 
           Pinging @elastic/ml-core (Team:ML)  | 
    
| return Void.TYPE; | ||
| }).when(client).execute(eq(GetDeploymentStatsAction.INSTANCE), eq(new GetDeploymentStatsAction.Request("test-deployment")), any()); | ||
| 
               | 
          ||
| safeSleep(1200); | 
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.
Is this waiting for the doAnswer() call? I'm not sure if this will be flaky but another option could be to add a count down latch and then here we await for the latch to be decremented.
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 is waiting for the adaptive allocations background process that makes these calls. That runs every 1s in this test (see constructor above). If within 1200ms nothing has happened, something is wrong.
With 200ms leeway it shouldn't be flaky, and never has been in the past.
I've also run these tests 100x locally without any problems.
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
…n inference adaptive allocations. (#125297)
…n inference adaptive allocations. (elastic#125297)
…n inference adaptive allocations. (elastic#125297)
…n inference adaptive allocations. (elastic#125297)
No description provided.