This repository was archived by the owner on Mar 9, 2022. It is now read-only.
Wait and check image pulling progress. #46
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
I tested the PR with On the client side: On the server side: |
mikebrow
approved these changes
May 24, 2017
| if err != nil { | ||
| return "", "", fmt.Errorf("failed to fetch image %q desc %+v: %v", ref, desc, err) | ||
| // Dispatch returns error when requested resources are locked. | ||
| // In that case, we should start waiting and checking the pulling |
Member
There was a problem hiding this comment.
so they are not always sync calls... :-) This explains some of the errors I was seeing... that the concurrent request may not be finished yet thus the locked resource!
Member
There was a problem hiding this comment.
Sort of wish they would block themselves.. but ok this works.
Member
Author
There was a problem hiding this comment.
@mikebrow so they are not always sync calls... :-) This explains some of the errors I was seeing... that the concurrent request may not be finished yet thus the locked resource!
The first call will be a sync call. The problem is that the second call returns immediately with error because someone else holds the lock of the resource. :)
Random-Liu
force-pushed
the
wait-image-pulling
branch
from
bf3fae0 to
49003c9
Compare
This was referenced May 24, 2017
Random-Liu
force-pushed
the
wait-image-pulling
branch
from
49003c9 to
d076706
Compare
yujuhong
reviewed
May 26, 2017
pkg/server/image_pull.go
Outdated
| } | ||
|
|
||
| // waitDownloadingInterval is the interval to check resource downloading progress. | ||
| const waitDownloadingInterval = 200 * time.Millisecond |
Member
There was a problem hiding this comment.
nit: waitDownloadingPollInterval might be better
pkg/server/image_pull.go
Outdated
| if err := c.waitForResourcesDownloading(ctx, resources.all()); err != nil { | ||
| return "", "", fmt.Errorf("failed to wait for image %q downloading: %v", ref, err) | ||
| } | ||
| glog.V(4).Infof("Finish downloading resources for image %q", ref) |
|
|
||
| // Unpack the image layers into snapshots. | ||
| if _, err = c.rootfsUnpacker.Unpack(ctx, manifest.Layers); err != nil { | ||
| rootfsUnpacker := rootfsservice.NewUnpackerFromClient(c.rootfsService) |
Member
Author
There was a problem hiding this comment.
Because in following PRs, we'll start to use rootfsService more, instead of only the Unpacker.
So I change the rootfsService to be a field of criContainerdService, and new a unpacker when we need it. The unpacker is only needed in this function.
Member
|
The rest is just minor nits. Feel free to apply the lgtm label and merge the PR after you update it. |
Member
Author
Signed-off-by: Random-Liu <[email protected]>
Signed-off-by: Random-Liu <[email protected]>
Random-Liu
force-pushed
the
wait-image-pulling
branch
from
d076706 to
c3ac5f7
Compare
Member
Author
|
Apply LGTM based on #46 (review) and #46 (review). |
lanchongyizu
pushed a commit
to lanchongyizu/cri-containerd
that referenced
this pull request
Sep 3, 2017
Wait and check image pulling progress.
adelina-t
pushed a commit
to adelina-t/cri
that referenced
this pull request
Sep 26, 2019
Don't panic when processing nil stats
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #41.
This PR:
Statusfunction in content api./cc @yujuhong @mikebrow