kvm: Agent should not check if remaining memory on host is sufficient#2766
Conversation
|
So, you are removing this validation from the KVM agent, right? |
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
as code cleanup it looks good, but how do we deal with memory deficiency if we don't check beforehand?
|
@rafaelweingartner Yes, it is being removed from the Agent. The old logic would sum up all the running VMs and then check if the remaining memory is enough. We however have a memory over commit ratio on the Management Server side which is not taken into account here. If the Management Server thinks (which is leading) that the Instance can start there, it should be started. Concurrency, I'm not aware. The mgmt server locks its tables, right? So you won't have those happening on the same host I'm assuming. |
|
Understood. Well, the MS should address concurrency, there are a few locking mechanisms in place, but sometimes I have the feeling that concurrency is not being addressed properly. Anyways, thanks for the PR. |
|
@blueorangutan package |
|
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✖centos7 ✔debian. JID-2207 |
When a Instance is (attempted to be) started in KVM Host the Agent should not worry about the allocated memory on this host. To make a proper judgement we need to take more into account: - Memory Overcommit ratio - Host reserved memory - Host overcommit memory The Management Server has all the information and the DeploymentPlanner has to make the decision if a Instance should and can be started on a Host, not the host itself. Signed-off-by: Wido den Hollander <wido@widodh.nl>
wido
force-pushed
the
kvm-agent-allocated-memory
branch
from
a4adb66 to
101c190
Compare
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
yadvr
left a comment
yadvr
left a comment
There was a problem hiding this comment.
I'm okay with the change, but not sure if this change may introduce a regression.
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2209 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
@rhtyd Thanks! As I stated, the Agent shouldn't worry about Memory Allocation is that's up to the Management Server and the deployment planners there. |
|
@rafaelweingartner Are you OK with me merging this PR as we have the approvals? |
|
Yes, I approved it already two days ago. Shouldn't we wait for the test results though? |
|
@rafaelweingartner Ah, yes, I didn't notice the tests yet :) Just wondering when we merge PRs into master right now. But indeed, let's wait for the tests. |
|
@blueorangutan test |
|
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
@blueorangutan package |
|
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2216 |
|
@blueorangutan test |
|
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2898)
|
7 participants
When a Instance is (attempted to be) started in KVM Host the Agent
should not worry about the allocated memory on this host.
To make a proper judgement we need to take more into account:
The Management Server has all the information and the DeploymentPlanner
has to make the decision if a Instance should and can be started on a
Host, not the host itself.
Signed-off-by: Wido den Hollander wido@widodh.nl