Update Linux Apt Commands to Handle Lock Timeouts #1861
Merged
Conversation
This file contains 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
This prevents us from having to run commands twice to get the status and stdout. Caution must be taken to make sure that the assumptions here in what is returned as stdout vs stderr are correct. Furthermore, when looking at the code, I saw something that looked just plain incorrect, so I will need to investigate that by adding the TODO before merge
…de-dotnet-runtime into nagilson-fix-eperm
This will allow us to not count times when users never respond to the installer as a failure and or remediate when the installer is taking too long.
We also pass the -y flag when it can help push through updates. Apt will return 100 whenever there is any failure. https://github.com/Debian/apt/blob/aa56836331870d975c212a5df2f13db9ce3914bf/apt-private/private-cmndline.cc#L602 This means the errors we are seeing are not just due to locking errors. But, perhaps this will help. Also this will try to run update twice, to see if that 2nd try will work. It seems a bit silly, but it does actually seem to improve the chance of success when something else is running on the system.
…ther failure message
…ral command execution
nagilson
commented
Jul 10, 2024
nagilson
commented
Jul 10, 2024
vscode-dotnet-runtime-library/src/Acquisition/InstallTrackerSingleton.ts
Outdated
Show resolved
Hide resolved
MiYanni
approved these changes
Jul 10, 2024
| { | ||
| await release(); | ||
| await lockfile.lock(lockPath, { retries: { retries: 10, minTimeout: 5, maxTimeout: 10000 } }) |
There was a problem hiding this comment.
What units are the min and max timeout in?
There was a problem hiding this comment.
they are in MS. The library will do an exponential equation where it starts trying again after 5 ms and then takes longer each retry until its at 10000 ms.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
resolves #1806 (comment)
A lot of users are seeing failure due to the DPKG lock being held by another process. We can add a flag to wait for the lock to be freed. This wont fix everything, as Apt will return 100 whenever there is any failure. https://github.com/Debian/apt/blob/aa56836331870d975c212a5df2f13db9ce3914bf/apt-private/private-cmndline.cc#L602
This means the errors we are seeing are not just due to locking errors. But, perhaps this will help.
This will also add a timeout to our own executions so that they dont keep the lock.
Also this will try to run update twice, to see if that 2nd try will work. It seems a bit silly, but it does actually seem to improve the chance of success when something else is running on the system.
The timeout time is mildly arbitrary.... we can try to improve that in the future to be based off a fraction of the total timeout time, or something like this. But I would like to see the impact of how much this helps without this change.