Skip to content
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

Performance drop for remove VCH operation #8267

Closed
gopakumarr opened this issue Sep 10, 2018 · 5 comments
Closed

Performance drop for remove VCH operation #8267

gopakumarr opened this issue Sep 10, 2018 · 5 comments
Labels
area/lifecycle Creation, management, and deletion of Virtual Container Hosts impact/doc/user Requires changes to official user documentation kind/defect/performance Behavior that is functionally correct, but performs worse than intended resolution/invalid The issue is intended behavior or otherwise invalid severity/3-moderate Medium usability or functional impact. Potentially has an inconvenient workaround. source/performance Reported by the performance testing team

Comments

@gopakumarr
Copy link

Uninstall/Remove VCH performance regression

From VIC engine build 19874 we are starting to observe Uninstall VIC command taking longer time neary +7 secs

Operation 19716 19874
Uninstall VCH 1.233 secs 8.134 secs

Deployment details

vic-machine-linux create --target $ESXHOSTIP --image-store $DATASTORE --name vic_$VERSION --compute-resource /ha-datacenter/host/*/Resources --no-tls --force --timeout 10m0s --user root --password mypassword

Uninstall VCH Command

vic-machine-linux delete --force --target $ESXHOSTIP --name vic_$VERSION --compute-resource /ha-datacenter/host/*/Resources --user root --password mypassword

From the logs it looks like time is mostly spent on removing portgroup

19716 logs snippet

INFO[0000] Removing volume stores
INFO[0000] Removing appliance VM network devices
INFO[0000] Removing Portgroup "vic_19716"
INFO[0001] Removing VirtualSwitch "vic_19716"
INFO[0001] Removing Resource Pool "vic_19716"

19874 logs snippet

INFO[0000] Removing volume stores
INFO[0000] Removing appliance VM network devices
INFO[0007] Removing Portgroup "vic_19874"
INFO[0007] Removing VirtualSwitch "vic_19874"
INFO[0008] Removing Resource Pool "vic_19874"

@zjs
Copy link
Member

zjs commented Sep 10, 2018

Could you attach debug-level logs for both versions? This would allow us to more precisely pinpoint a possible cause.

@zjs zjs added kind/defect/performance Behavior that is functionally correct, but performs worse than intended severity/3-moderate Medium usability or functional impact. Potentially has an inconvenient workaround. area/lifecycle Creation, management, and deletion of Virtual Container Hosts source/performance Reported by the performance testing team status/need-info Additional information is needed to make progress labels Sep 10, 2018
@hickeng
Copy link
Member

hickeng commented Sep 10, 2018

The regression was almost certainly introduced as a legitimate change in build 19750. Unfortunately the CI network infrastructure issues (vmware/vic-tasks#69) prevented an updated build from publishing cleanly until 19874 so there's a set of 15 commits that present in 19874.

The change I think introduces this is #6943 which adds the polite guest shutdown path instead of direct power off, meaning we're waiting for the vic-engine components to finalize (including logging out of VC), OS services to shut down, and the OS to power off via ACPI. 8 seconds is in line with what I observed during testing for this change.

@gopakumarr apologies - I should have flagged this change to you as a known impact.

@hickeng hickeng closed this as completed Sep 10, 2018
@zjs zjs added resolution/invalid The issue is intended behavior or otherwise invalid impact/doc/note Requires creation of or changes to an official release note and removed status/need-info Additional information is needed to make progress labels Sep 10, 2018
@zjs
Copy link
Member

zjs commented Sep 13, 2018

This may be worth release noting as it's an expected user-visible performance change. Perhaps the release note can include some information about #6943 as well, for context.

@stuclem stuclem added impact/doc/user Requires changes to official user documentation and removed impact/doc/note Requires creation of or changes to an official release note labels Sep 17, 2018
@stuclem
Copy link
Contributor

stuclem commented Sep 17, 2018

@zjs, does this mean that this longer shutdown time is now the standard behaviour? If so, then I think that it needs to go into the core docs rather than in a release note.

@zjs
Copy link
Member

zjs commented Sep 18, 2018

does this mean that this longer shutdown time is now the standard behaviour?

Yes.

If so, then I think that it needs to go into the core docs rather than in a release note.

I couldn't find anything in the current docs about expected duration of the shutdown operation. (And that seems like a generally difficult thing to characterize, so it's probably best not to try to document it.)

It seemed like it may be worth noting in the context of, essentially: after you upgrade, this operation will take longer, and that's not a sign that anything is wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/lifecycle Creation, management, and deletion of Virtual Container Hosts impact/doc/user Requires changes to official user documentation kind/defect/performance Behavior that is functionally correct, but performs worse than intended resolution/invalid The issue is intended behavior or otherwise invalid severity/3-moderate Medium usability or functional impact. Potentially has an inconvenient workaround. source/performance Reported by the performance testing team
Projects
None yet
Development

No branches or pull requests

4 participants