Skip to content

Add timeouts for volume plugin ops#35441

Merged
thaJeztah merged 1 commit intomoby:masterfrom
cpuguy83:plugin_timeout
Jan 17, 2018
Merged

Add timeouts for volume plugin ops#35441
thaJeztah merged 1 commit intomoby:masterfrom
cpuguy83:plugin_timeout

Conversation

@cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Nov 8, 2017

This protects the daemon from volume plugins that are slow or
deadlocked.

These are some fairly conservative timeouts, but I still worry that they could be problematic.
ping @tonistiigi

Copy link
Member

Choose a reason for hiding this comment

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

if this function retries with backoff then it shouldn't probably reuse the context as just waiting for a backoff will cause this context to close

@cpuguy83 cpuguy83 force-pushed the plugin_timeout branch 2 times, most recently from d80336e to 130dde7 Compare December 7, 2017 20:10
@thaJeztah
Copy link
Member

@tonistiigi looks like the PR was updated PTAL

Copy link
Member

Choose a reason for hiding this comment

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

Is it documented that request context does not have any effect after Do returns? Technically the request is still active at this point.

@cpuguy83 cpuguy83 force-pushed the plugin_timeout branch 9 times, most recently from c58f11f to f1cf97e Compare January 10, 2018 01:43
@cpuguy83
Copy link
Member Author

Ok, I've updated this. PTAL.

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

nit: why is this inside the loop?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, from prior implementation attempt. Will fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

This protects the daemon from volume plugins that are slow or
deadlocked.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit f0b0f20 into moby:master Jan 17, 2018
@cpuguy83 cpuguy83 deleted the plugin_timeout branch January 23, 2018 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants