Skip to content

Maintenance: Check Stopping before reacting to error code#1064

Merged
derrickstolee merged 1 commit intomicrosoft:masterfrom
derrickstolee:check-stopping
Apr 24, 2019
Merged

Maintenance: Check Stopping before reacting to error code#1064
derrickstolee merged 1 commit intomicrosoft:masterfrom
derrickstolee:check-stopping

Conversation

@derrickstolee
Copy link
Contributor

When someone unmounts, it triggers a Stop() on the maintenance
step that is currently running. This can interrupt longer-running Git
commands more often than other actions, so check Stopping before
checking the error code for a response.

This is more likely a reason for people to hit this error than any
data-shape issues that we were expecting to cause a problem.

While I am updating the check around the commit-graph verify
command, I am not un-commenting it as we don't have the full
fix for the "too many pack-files" case.

When someone unmounts, it triggers a Stop() on the maintenance
step that is currently running. This can interrupt longer-running Git
commands more often than other actions, so check Stopping before
checking the error code for a response.
@derrickstolee derrickstolee requested a review from jeschu1 April 24, 2019 14:46
@derrickstolee derrickstolee merged commit 7736d15 into microsoft:master Apr 24, 2019
derrickstolee added a commit that referenced this pull request Apr 26, 2019
…Git is running

The GitMaintenanceStep throws a StoppingException if the
RunGitCommand() method is run after a Stop() call. However,
it doesn't throw when the Stop() call happens during the
Git process. This requires extra handling of the Stopping
boolean after receiving a response from RunGitCommand().
Such extra handling was added in #1064, but would not
be necessary with this change.

The better thing to do is to throw the StoppingException
after the Git process completes.

Add a unit test that verifies we automatically skip the
remainder of a PerformMaintenance() method in a subclass.

Looking through the error logs, all logged instances of a
failure during 'commit-graph verify' or 'multi-pack-index verify'
are actually due to this issue. Otherwise we would see errors
from GitMaintenanceStep saying "Git process {command} failed with errors:"
and those messages do no appear in the logs.
@jrbriggs jrbriggs added this to the M151 milestone May 1, 2019
@jrbriggs jrbriggs added affects: live-site Improving our ability to diagnose and fix the product platform: windows platform: macOS type: bug labels May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

affects: live-site Improving our ability to diagnose and fix the product platform: macOS platform: windows type: bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants