-
Notifications
You must be signed in to change notification settings - Fork 257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add retries when removing device mapper target #1200
Conversation
15947ea
to
0794abb
Compare
0794abb
to
c3b802a
Compare
|
||
// This is workaround for "device or resource busy" error, which occasionally happens after the device mapper | ||
// target has been unmounted. | ||
for i := 0; i < 10; i++ { | ||
if err = rm(); err != nil { | ||
time.Sleep(10 * time.Millisecond) | ||
continue | ||
} | ||
break | ||
} | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we still be checking to see if the context is cancelled even though we have a limited number of tries here? I'm not sure what makes sense to do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt we'll ever hit a context cancellation/timeout since the combined retry time is relatively short (~100ms) compared to the default context timeout. Should we even consider an outside context within this function? I'd think not?
Additionally ignore the error from device mapper if target removal still fails. The corresponding layer path is already unmounted at that point and this avoids having an inconsistent state. Signed-off-by: Maksim An <maksiman@microsoft.com>
Signed-off-by: Maksim An <maksiman@microsoft.com>
Signed-off-by: Maksim An <maksiman@microsoft.com>
c3b802a
to
ad1b044
Compare
Signed-off-by: Maksim An <maksiman@microsoft.com>
@msscotb added a unit test. @dcantah, @katiewasnothere PTAL as well |
Signed-off-by: Maksim An <maksiman@microsoft.com>
Signed-off-by: Maksim An <maksiman@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Actually, re-reading what error is returned from ioctl() (the |
Signed-off-by: Maksim An <maksiman@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok last approval I swear 😂
cmon, it's a great presence of mind you got going there! 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm again
Add retries when removing device mapper target Additionally ignore the error from device mapper if target removal still fails. The corresponding layer path is already unmounted at that point and this avoids having an inconsistent state. Signed-off-by: Maksim An <maksiman@microsoft.com>
Related work items: microsoft#1067, microsoft#1097, microsoft#1119, microsoft#1170, microsoft#1176, microsoft#1180, microsoft#1181, microsoft#1182, microsoft#1183, microsoft#1184, microsoft#1185, microsoft#1186, microsoft#1187, microsoft#1188, microsoft#1189, microsoft#1191, microsoft#1193, microsoft#1194, microsoft#1195, microsoft#1196, microsoft#1197, microsoft#1200, microsoft#1201, microsoft#1202, microsoft#1203, microsoft#1204, microsoft#1205, microsoft#1206, microsoft#1207, microsoft#1209, microsoft#1210, microsoft#1211, microsoft#1218, microsoft#1219, microsoft#1220, microsoft#1223
Add retries when removing device mapper target Additionally ignore the error from device mapper if target removal still fails. The corresponding layer path is already unmounted at that point and this avoids having an inconsistent state. Signed-off-by: Maksim An <maksiman@microsoft.com>
Additionally ignore the error from device mapper if target removal
still fails. The corresponding layer path is already unmounted
at that point and this avoids having an inconsistent state.
Signed-off-by: Maksim An maksiman@microsoft.com