Skip to content
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

Fix for #1743 - do not exit early if no blobs found for a period when prefetching. #1745

Merged
merged 2 commits into from
Jul 14, 2021

Conversation

tyrielv
Copy link
Contributor

@tyrielv tyrielv commented Jun 28, 2021

Fix for #1743 - do not exit early if no blobs found for a period when prefetching.

@derrickstolee
Copy link
Contributor

@tyrielv: did you build and install this locally to verify that it fixes your larger scenario? I'm not seeing a clear reason why this fixes your case.

@tyrielv
Copy link
Contributor Author

tyrielv commented Jun 28, 2021

I'm still working on verifying it, but here's the reasoning:

If it takes more than ChunkSize * TimeoutMs (ie about 7 minutes) for the missingBlobs collection to produce its first value, then the loop currently exits and returns false. If it returns false, then the while loop on line 74 never runs and won't try again, even though the producer for missingBlobs is still running.

In my scenario it took about 1 hour for the missingBlobs producer to finish running and it only discovered 3 missing blobs out of 1.6 million, so it is very likely that there was at least one 7 minute gap before the 3 blobs were all discovered.

(It's considerably less likely on the machine I'm trying to repro on, which only takes about 10 minutes instead of 60 to check all the blobs)

@derrickstolee
Copy link
Contributor

I'm still working on verifying it, but here's the reasoning:

If it takes more than ChunkSize * TimeoutMs (ie about 7 minutes) for the missingBlobs collection to produce its first value, then the loop currently exits and returns false. If it returns false, then the while loop on line 74 never runs and won't try again, even though the producer for missingBlobs is still running.

I see. I now can connect the dots that the early stage is really slow in some cases.

In my scenario it took about 1 hour for the missingBlobs producer to finish running and it only discovered 3 missing blobs out of 1.6 million, so it is very likely that there was at least one 7 minute gap before the 3 blobs were all discovered.

And I didn't expect this to be so slow, but I suppose it would be when checking 1.6 million blobs via libgit2. I'm more convinced that this is a fix, but I appreciate you doing the extra testing.

We won't want to merge this until after #1744 is merged, anyway.

@derrickstolee
Copy link
Contributor

@tyrielv: #1744 is merged. Could you update your branch to kick off a new merge? That will allow us to run the PR validation builds. One easy way is to run git commit --amend --no-edit && git push -f <remote> issue1743

@tyrielv
Copy link
Contributor Author

tyrielv commented Jul 14, 2021

@derrickstolee can you approve the workflow so this can run tests?

@derrickstolee derrickstolee merged commit 07e8cb4 into microsoft:master Jul 14, 2021
derrickstolee added a commit that referenced this pull request Jul 15, 2021
Major Updates
----------------

* Comes with `microsoft/git` v2.32.0.vfs.0.4.
* This release will be available via `winget`.
* The `gvfs upgrade` verb is deprecated. Any future updates will be taken via `winget`.
* This version of Git has an upgrade mechanism that can advance independently of VFS for Git. Run `git update-microsoft-git` to update your Git version.
* The build system has been updated to use a more recent version of .NET.
* All macOS and POSIX code has been removed to simplify the build system.
* A FastFetch bug around deleting files has been fixed.
* A prefetch bug around downloading a small number of files has been fixed.

Special thanks to contributors @ldennington, @tyrielv, @50Wliu, and @SteveBenz.

Pull Requests
---------------

* #1738: Bumping version of update-winget action
* #1747: UpgradeVerb: write deprecation notice
* #1745: do not exit early if no blobs found for a period when prefetching
* #1740: Skip launching UI if running unattended
* #1737: Update Git to v2.32.0
* #1746: Delete custom upgrader
* #1741: Fix winget tag specification
* #1744: Overhaul build and project systems and drop Mac/POSIX code
* #1736: Update Git to v2.31.1.vfs.0.3
* #1735: Bumping winget action version
* #1734: Adding winget workflow
* #1733: Update Git to include 2.31.1
* #1730: Fix an issue with FastFetch when deleting files
* #1732: Use one NuGet feed
* #1726: Config: commitGraph.generationVersion=1
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.

2 participants