Add timeout for send and receive#170
Conversation
aboch
changed the title
|
+1 |
|
Seems a little funky that this is implemented using just go with a timeout waiting for the goroutine to return rather on the socket itself. |
nl/nl_linux.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| case <-time.After(req.Timeout): |
There was a problem hiding this comment.
this will leak a lot of timers. You should use NewTimer and stop them.
handle_linux.go
Outdated
| // SetTimeout sets the amount of time the library will wait for the socket send | ||
| // and for the socket receive on this handle to terminate before declaring a | ||
| // a timeout failure to the client. | ||
| func (h *Handle) SetTimeout(to time.Duration) { |
There was a problem hiding this comment.
I'd better go with NewHandleWithTimeout to avoid mutexes. But it's up to @vishvananda for decide of course.
There was a problem hiding this comment.
I wanted to avoid a precedence for adding a new constructor each time we add a new option.
This is more flexible and client will likely set the timeout once at handle creation. After that it is only going to be read locks.
nl/nl_linux.go
Outdated
| select { | ||
| case err := <-rch: | ||
| return res, err | ||
| case <-time.After(req.Timeout): |
There was a problem hiding this comment.
I think send and read should be in the same goroutine for having one timeout for whole Execute and simplify code.
There was a problem hiding this comment.
I have seen reports where the hung happen in both the send and receive phase, I wanted the client to know during which phase the timeout happened.
|
@cpuguy83 yeah, there is |
Signed-off-by: Alessandro Boch <[email protected]>
|
🎉 thanks @vishvananda! |
|
it may a kernel bug. you can try insert a probe into the count will be stable temporary , you will the its different between the number of call and return . |
5 participants
Because of kernel bugs, we have seen cases where the call to netlink socket send or receive will wait indefinitely causing docker engine to hang.
This PR allows client to set a timeout for socket send and receive for a netlink handle via
Handle.SetTimeout(), in order to gracefully workaround the above situation.A timeout logic was also requested in #131
Signed-off-by: Alessandro Boch [email protected]