Failed to stop container: lock failed

[fankangbest]

Ⅰ. Issue Description

When I run e2e test case for kubelet in kubernetes, and there is a case sometimes failed with error:
Sep 28 18:49:37.028: INFO: [WaitTimeoutForContainerUpdateStatus] updateStatusStr: {"statuses":{"pod-base":{"creationTimestamp":"2018-09-28T18:49:30.096971955+08:00","finishTimestamp":"2018-09-28T18:49:30.098021602+08:00","currentState":"running","lastState":"running","action":"stop","message":"killContainer "pod-base"(id={"pouch" "c69994fce9611e75ca826c644edb8f4dbc32a2592afb66dd34e1d4a2d9a76c0e"}) for pod "createpodtest0d0c65b1-c30c-11e8-b2a9-563fceceea99_e2e-tests-sigma-kubelet-5wz59(0d0d1072-c30c-11e8-9e55-563fceceea99)" failed: rpc error: code = Unknown desc = failed to stop container "c69994fce9611e75ca826c644edb8f4dbc32a2592afb66dd34e1d4a2d9a76c0e": failed to destroy container c69994fce9611e75ca826c644edb8f4dbc32a2592afb66dd34e1d4a2d9a76c0e: lock failed"}}}
Maybe pouch locks the container fails when kubelet try to stop a container.

Ⅱ. Describe what happened

Ⅲ. Describe what you expected to happen

Ⅳ. How to reproduce it (as minimally and precisely as possible)

Ⅴ. Anything else we need to know?

Ⅵ. Environment:

Pouch GitCommit: bdab411

[allencloud]

I am afraid that code

// DestroyContainer kill container and delete it.
func (c *Client) destroyContainer(ctx context.Context, id string, timeout int64) (*Message, error) {
.......

	if !c.lock.Trylock(id) {
		return nil, errtypes.ErrLockfailed
	}
	defer c.lock.Unlock(id)

	pack, err := c.watch.get(id)
	if err != nil {
		return nil, err
	}
......

Would affect this. When reading the code, we found that a specific container is only allowed to be executed by one operation, like stop, start and so on.

When a container is executed by one operation, the other one which wishes to get the container would return the lock failure error.

So when there are many concurrent accesses to the container, the lock failed error happens.

[allencloud]

We have three proposals here:

return ErrLockfailed directly if lock fails;

Currently we did this in the code master branch.

wait until the goroutine get the lock

I think this would block lots of operation request.

return ErrLockfailed if lock fails and wait for a interval

this is a tradeoff.

@fankangbest

[pouchrobot]

Thanks for your report, @fankangbest
😱 This is a priority/P1 issue which is highest.
Seems to be severe enough.
ping @alibaba/pouch , PTAL.

[SimonCqk]

We have three proposals here:

return ErrLockfailed directly if lock fails;

Currently we did this in the code master branch.

wait until the goroutine get the lock

I think this would block lots of operation request.

return ErrLockfailed if lock fails and wait for a interval

this is a tradeoff.

@fankangbest

Why not add maxTryTimes and return ErrLockfailed if all fail out ?

[allencloud]

Why not add maxTryTimes and return ErrLockfailed if all fail out ?

@SimonCqk This is the most straight forward way, and we have decided to do this temporarily. See #2287 (comment)