Fix for #1743 - do not exit early if no blobs found for a period when prefetching. by tyrielv · Pull Request #1745 · microsoft/VFSForGit (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits2 Checks0 Files changed

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

[ Show hidden characters]({{ revealButtonHref }})

tyrielv

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

@tyrielv

@derrickstolee

@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

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

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

@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

@tyrielv

derrickstolee

derrickstolee added a commit that referenced this pull request

Jul 15, 2021

@derrickstolee

Major Updates

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

Pull Requests

2 participants

@tyrielv @derrickstolee