Use backoff to tolerant race condition. #1894
Merged
Conversation
This file contains 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
dashpole
reviewed
Feb 23, 2018
container/containerd/handler.go
Outdated
| // `ContainerStatus` only returns result after `StartContainer` finishes. | ||
| var taskPid uint32 | ||
| backoff := 100 * time.Millisecond | ||
| for retry := 5; retry > 0; retry-- { |
There was a problem hiding this comment.
so after 5 retries we continue on? Will we not get any metrics in this case? Would it be better just to return an error so it doesnt fail silently?
Signed-off-by: Lantao Liu <lantaol@google.com>
Random-Liu
force-pushed
the
avoid-containerd-race
branch
from
f484c62 to
d5ee05f
Compare
dashpole
reviewed
Feb 23, 2018
| } | ||
| retry-- | ||
| if !errdefs.IsNotFound(err) || retry == 0 { | ||
| return nil, err |
There was a problem hiding this comment.
If retry == 0 case is hit, we will return err = nil. This will probably result in a nil pointer somewhere down the road. Can we return a new error for this case?
There was a problem hiding this comment.
No, we check err == nil before this, right?
k8s-github-robot
pushed a commit
to kubernetes/kubernetes
that referenced
this pull request
Mar 8, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Update cadvisor to v0.29.1 Update cadvisor to v0.29.1 to include a bug fix for containerd integration. google/cadvisor#1894 **Release note**: ```release-note none ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I keep seeing this error in kubelet log:
The reason is that container cgroup is created in the middle of task creation. And there is a race condition that cadvisor see the cgroup, but the corresponding task hasn't been fully created yet in containerd.
There is no such problem for docker, because docker has some internal lock to make sure
Inspectonly returns after container start is finished. CRI-Containerd and I believe cri-o are doing the same thing. Actually cadvisor is relying on some container runtime internal implementation details here.However, here we are talking with containerd directly, which still has this race condition.
This PR added a retry to avoid this problem for now, and we should come up with a better fix next release.
I've validated this PR, and it works for me.
/cc @abhi @dashpole
Signed-off-by: Lantao Liu lantaol@google.com