Add retries when removing device mapper target #1200
Merged
Conversation
This file contains hidden or 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
anmaxvl
force-pushed
the
device-mapper-retries
branch
from
15947ea to
0794abb
Compare
dcantah
reviewed
Oct 19, 2021
dcantah
reviewed
Oct 19, 2021
anmaxvl
force-pushed
the
device-mapper-retries
branch
from
0794abb to
c3b802a
Compare
dcantah
approved these changes
Oct 22, 2021
Comment on lines
283
to
294
|
|
||
| // 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.
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
Contributor
Author
There was a problem hiding this comment.
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?
msscotb
reviewed
Oct 29, 2021
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 <[email protected]>
Signed-off-by: Maksim An <[email protected]>
Signed-off-by: Maksim An <[email protected]>
anmaxvl
force-pushed
the
device-mapper-retries
branch
from
c3b802a to
ad1b044
Compare
Signed-off-by: Maksim An <[email protected]>
Contributor
Author
|
@msscotb added a unit test. @dcantah, @katiewasnothere PTAL as well |
dcantah
reviewed
Nov 1, 2021
Signed-off-by: Maksim An <[email protected]>
dcantah
reviewed
Nov 1, 2021
Signed-off-by: Maksim An <[email protected]>
Contributor
|
Actually, re-reading what error is returned from ioctl() (the |
Signed-off-by: Maksim An <[email protected]>
dcantah
approved these changes
Nov 2, 2021
Contributor
dcantah
left a comment
dcantah
left a comment
There was a problem hiding this comment.
ok last approval I swear 😂
Contributor
Author
cmon, it's a great presence of mind you got going there! 😄 |
helsaawy
pushed a commit
to helsaawy/hcsshim
that referenced
this pull request
Nov 3, 2021
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 <[email protected]>
anmaxvl
pushed a commit
to anmaxvl/hcsshim
that referenced
this pull request
Nov 17, 2021
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
princepereira
pushed a commit
to princepereira/hcsshim
that referenced
this pull request
Aug 29, 2024
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 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
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 [email protected]