kubernetes / minikube

Run Kubernetes locally
https://minikube.sigs.k8s.io/
Apache License 2.0
29.05k stars 4.86k forks source link

Improve Hyper-V performance by reduce duplicate Hyper-V operations #10136

Open lingsamuel opened 3 years ago

lingsamuel commented 3 years ago

See #10135

docker-env and status need 10s in average. Seems like each Hyper-V operation (get running state, get ip addr) takes 2s+.

Comparison: Linux OS < 1s

/kind feature

lingsamuel commented 3 years ago

/label os/windows

k8s-ci-robot commented 3 years ago

@lingsamuel: The label(s) /label os/windows cannot be applied. These labels are supported: api-review, community/discussion, community/maintenance, community/question, cuj/build-train-deploy, cuj/multi-user, platform/aws, platform/azure, platform/gcp, platform/minikube, platform/other, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash

In response to [this](https://github.com/kubernetes/minikube/issues/10136#issuecomment-759306070): >/label os/windows Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
medyagh commented 3 years ago

thats a good idea @lingsamuel if indeed we doing those operations multiple times. could we verify how many times we actually call those funcs ? maybe adding some debug.Printstack() and them and see how many times we call it and reduce those calls

lingsamuel commented 3 years ago

From the log I pasted #10135 we can say it repeat .state at least 3 times and get ip address twice

lingsamuel commented 3 years ago

The latency only happens when we are accessing host.Driver.XXX. I suspect the reason is powershell starts slowly.

    host, _ := api.Load(machineName)
    s, _ := host.Driver.GetState()

Slow operation is Driver.GetState().

In status command:

func nodeStatus(api libmachine.API, cc config.ClusterConfig, n config.Node) (*Status, error) {
// ...
    hs, err := machine.Status(api, name)
// ...
    if _, err := cluster.DriverIP(api, name); err != nil {
// .....
    host, err := machine.LoadHost(api, name)
    cr, err := machine.CommandRunner(host)
//...
lingsamuel commented 3 years ago

Some libmachine codes: GetIP, GetState, GetSSHHostname()

From the code we know machine-drivers/machine doesn't cache anything.

lingsamuel commented 3 years ago

The strange thing is the lib already set IP inside Start(). Is there any reason to retrieve IP every time?

lingsamuel commented 3 years ago

By this simple patch, docker-env and status only need 2s currently (1 State call).

lingsamuel commented 3 years ago

xref machine-drivers/machine#31

lingsamuel commented 3 years ago

Can we use a fork version with this patch https://github.com/machine-drivers/machine/pull/32? @medyagh

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

fejta-bot commented 3 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten

lingsamuel commented 3 years ago

/remove-lifecycle rotten

lingsamuel commented 3 years ago

Any updates? I think I've provided enough information to confirm this. /ping @medyagh

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

sharifelgamal commented 2 years ago

I believe @afbjorklund has write access to machine-drivers/machine, hopefully he can merge your PR there and we can update the reference in our code here.