Skip to content
This repository was archived by the owner on Apr 3, 2018. It is now read-only.

pod: Improve pod create performances - #484

Merged
sboeuf merged 1 commit into
masterfrom
sboeuf/improve_start_vm
Nov 14, 2017
Merged

pod: Improve pod create performances#484
sboeuf merged 1 commit into
masterfrom
sboeuf/improve_start_vm

Conversation

@sboeuf

@sboeufsboeuf commented Nov 14, 2017

Copy link
Copy Markdown
Collaborator

pod: Improve pod create performances

In order to optimize pod creation, this patch does not wait for the
VM after it has been started. Instead, it will wait for it during the
"start" stage. This will allow our VM to be started while the caller
could potentially do other things between create and start. Globally,
this improves the boot time of our containers.

@sboeuf
sboeufforce-pushed the sboeuf/improve_start_vm branch 2 times, most recently from 71f8940 to 918b7b1CompareNovember 14, 2017 15:58
Comment threadpod.go Outdated
vmStartedCh := make(chan error)
const timeout = time.Duration(10) * time.Second

go p.hypervisor.waitPod(vmStartedCh)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we simply make waitPod blocking and pass it a timeout instead?

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I can do that. The reason I have chosen such an implementation is to avoid the duplication of timeout handling for every hypervisor implementation.

Comment threadqemu.go

q.Logger().WithError(err).Warn("Failed to connect to QEMU instance, retrying...")
time.Sleep(time.Duration(50) * time.Millisecond)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This potentially is an infinite loop. By giving waitPod a timeout, you could implement a non infinte loop here based on the timeout argument.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree, but I thought that every hypervisor implementation will have to implement the same mechanism which is duplication. I know this could be an infinite loop but when the main process returns, this go routine is gonna be terminated.

Comment threadqemu.go Outdated
break
}

q.Logger().WithError(err).Warn("Failed to connect to QEMU instance, retrying...")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not print a new log entry every 50ms, but print one if we timeout instead.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change that.

@sboeuf

Copy link
Copy Markdown
CollaboratorAuthor

@sameo I am gonna make the changes ;)

@sboeuf
sboeufforce-pushed the sboeuf/improve_start_vm branch from 918b7b1 to 2f3541cCompareNovember 14, 2017 18:10
@sameo

sameo commented Nov 14, 2017

Copy link
Copy Markdown
Collaborator

LGTM

Approved with PullApproveApproved with PullApprove

@sboeuf

Copy link
Copy Markdown
CollaboratorAuthor

@sameo comments addressed !

In order to optimize pod creation, this patch does not wait for the
VM after it has been started. Instead, it will wait for it during the
"start" stage. This will allow our VM to be started while the caller
could potentially do other things between create and start. Globally,
this improves the boot time of our containers.
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
@sboeuf
sboeufforce-pushed the sboeuf/improve_start_vm branch from 2f3541c to b20ba87CompareNovember 14, 2017 18:37
@sboeuf
sboeuf merged commit a169025 into masterNov 14, 2017
@sameo
sameo deleted the sboeuf/improve_start_vm branch December 18, 2017 14:14
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@sboeuf@sameo