Skip to content

Always index locally for prefetch packs #1840

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 13, 2025
Merged

Always index locally for prefetch packs #1840

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 34 additions & 45 deletions GVFS/GVFS.Common/Git/GitObjects.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@
foreach (PrefetchPacksDeserializer.PackAndIndex pack in deserializer.EnumeratePacks())
{
// The advertised size may not match the actual, on-disk size.
long indexLength = 0;

Check warning on line 683 in GVFS/GVFS.Common/Git/GitObjects.cs

View workflow job for this annotation

GitHub Actions / Build and Unit Test (Debug) 

The variable 'indexLength' is assigned but its value is never used

Check warning on line 683 in GVFS/GVFS.Common/Git/GitObjects.cs

View workflow job for this annotation

GitHub Actions / Build and Unit Test (Release) 

The variable 'indexLength' is assigned but its value is never used
long packLength;

// Write the temp and index to a temp folder to avoid putting corrupt files in the pack folder
Expand All @@ -706,64 +706,53 @@

bytesDownloaded += packLength;

// We will try to build an index if the server does not send one
if (pack.IndexStream == null)
// We can't trust the index file from the server, so we will always build our own.
// We still need to consume and handle any exceptions from the index stream though.
Comment on lines +709 to +710
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we may want to add an option to use the pack index as given by the server (default to ignore it of course) for performance critical scenarios where we can also trust the server.

For now let's just be safe and never trust the pack index, and can add this in the future if needed.

var canContinue = true;
GitProcess.Result result;
if (this.TryBuildIndex(activity, packTempPath, out result, gitProcess))
{
GitProcess.Result result;
if (!this.TryBuildIndex(activity, packTempPath, out result, gitProcess))
tempPacks.Add(new TempPrefetchPackAndIdx(pack.Timestamp, packName, packTempPath, packFlushTask, idxName, idxTempPath, idxFlushTask: null));
if (pack.IndexStream != null)
{
if (packFlushTask != null)
try
{
packFlushTask.Wait();
bytesDownloaded += pack.IndexStream.Length;
if (pack.IndexStream.CanSeek)
{
pack.IndexStream.Seek(0, SeekOrigin.End);
}
else
{
pack.IndexStream.CopyTo(Stream.Null);
}
}
catch (Exception e)
{
canContinue = false;
EventMetadata metadata = CreateEventMetadata(e);
activity.RelatedWarning(metadata, "Failed to read to end of index stream");
}

// Move whatever has been successfully downloaded so far
Exception moveException;
this.TryFlushAndMoveTempPacks(tempPacks, ref latestTimestamp, out moveException);

return new RetryWrapper<GitObjectsHttpRequestor.GitObjectTaskResult>.CallbackResult(null, true);
}

tempPacks.Add(new TempPrefetchPackAndIdx(pack.Timestamp, packName, packTempPath, packFlushTask, idxName, idxTempPath, idxFlushTask: null));
}
else
{
Task indexFlushTask;
if (this.TryWriteTempFile(activity, pack.IndexStream, idxTempPath, out indexLength, out indexFlushTask))
canContinue = false;
}

if (!canContinue)
{
if (packFlushTask != null)
{
tempPacks.Add(new TempPrefetchPackAndIdx(pack.Timestamp, packName, packTempPath, packFlushTask, idxName, idxTempPath, indexFlushTask));
packFlushTask.Wait();
}
else
{
bytesDownloaded += indexLength;

// Try to build the index manually, then retry the prefetch
GitProcess.Result result;
if (this.TryBuildIndex(activity, packTempPath, out result, gitProcess))
{
// If we were able to recreate the failed index
// we can start the prefetch at the next timestamp
tempPacks.Add(new TempPrefetchPackAndIdx(pack.Timestamp, packName, packTempPath, packFlushTask, idxName, idxTempPath, idxFlushTask: null));
}
else
{
if (packFlushTask != null)
{
packFlushTask.Wait();
}
}

// Move whatever has been successfully downloaded so far
Exception moveException;
this.TryFlushAndMoveTempPacks(tempPacks, ref latestTimestamp, out moveException);
// Move whatever has been successfully downloaded so far
Exception moveException;
this.TryFlushAndMoveTempPacks(tempPacks, ref latestTimestamp, out moveException);

// The download stream will not be in a good state if the index download fails.
// So we have to restart the prefetch
return new RetryWrapper<GitObjectsHttpRequestor.GitObjectTaskResult>.CallbackResult(null, true);
}
return new RetryWrapper<GitObjectsHttpRequestor.GitObjectTaskResult>.CallbackResult(null, true);
}

bytesDownloaded += indexLength;
}

Exception exception = null;
Expand Down
Loading