Skip to content

fixed win32 p_unlink retry sleep issue#4312

Closed
cjhoward92 wants to merge 3 commits into
libgit2:masterfrom
cjhoward92:fix/win32_p_unlink_retry_hang
Closed

fixed win32 p_unlink retry sleep issue#4312
cjhoward92 wants to merge 3 commits into
libgit2:masterfrom
cjhoward92:fix/win32_p_unlink_retry_hang

Conversation

@cjhoward92
Copy link
Copy Markdown
Contributor

@cjhoward92 cjhoward92 commented Jul 18, 2017

Fixed an issue where the retry logic on p_unlink in posix_w32.c sleeps before it tries setting a file to write mode causing unnecessary slowdown.

This affects windows only.

The issue was introduced in this commit as the do_with_retries macro tries the unlink_once routine and sleeps on failure before calling ensure_writable.

This is particularly noticeable when staging large groups of files at once as staging creates a temporary git object that is set to read only. When this temporary git object is deleted via p_unlink it always ends up hitting the sleep causing a 5ms delay per file.

I spoke with @ethomson on slack about this briefly.

Carson Howard added 3 commits July 18, 2017 14:47
Fixed an issue where the retry logic on p_unlink sleeps before it tries setting a file to write mode causing unnecessary slowdown.
@cjhoward92
Copy link
Copy Markdown
Contributor Author

Did not notice this PR.

@cjhoward92 cjhoward92 closed this Jul 18, 2017
@pks-t
Copy link
Copy Markdown
Member

pks-t commented Jul 21, 2017

Heh, you've been late by only 5 minutes. Thanks anyway for your PR!

@cjhoward92
Copy link
Copy Markdown
Contributor Author

@pks-t Yeah! I was little bummed but that's okay as long as it gets fixed! Thank you for helping to maintain the library! It's great.

ethomson added a commit that referenced this pull request Jul 26, 2017
When using the `do_with_retries` macro for retrying filesystem
operations in the posix emulation layer, allow the remediation function
to return `GIT_RETRY`, meaning that the error was believed to be
remediated, and the operation should be retried immediately, without
a sleep.

This is a slightly more general solution to the problem fixed in #4312.
@ethomson
Copy link
Copy Markdown
Member

Yeah, sorry again for the poor communication on my part @cjhoward92. I'm manually merging your pull request into my branch, because I want to make sure that your change gets in to the code base. 😀 I've rebased my change on top, since I think that it's a little bit more general-purpose, and I'll merge them both in #4311.

Thanks again for the investigation and the fix! 🎉

@cjhoward92
Copy link
Copy Markdown
Contributor Author

Wow thanks @ethomson! Appreciate it!

swisspol pushed a commit to git-up/libgit2 that referenced this pull request Feb 4, 2018
When using the `do_with_retries` macro for retrying filesystem
operations in the posix emulation layer, allow the remediation function
to return `GIT_RETRY`, meaning that the error was believed to be
remediated, and the operation should be retried immediately, without
a sleep.

This is a slightly more general solution to the problem fixed in libgit2#4312.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants