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 object not found error in OFS deltas #326

Closed

Conversation

zeripath
Copy link
Contributor

Unfortunately the code in #303 was incorrect for large objects in OFS deltas.

The underlying fault is that the seeker in the packfile was seeking away whilst the
delta was being read.

This PR simply reads the delta into a buffer.

Fix #323

Signed-off-by: Andrew Thornton art27@cantab.net

Unfortunately the code in go-git#303 was incorrect for large objects in OFS deltas.

The underlying fault is that the seeker in the packfile was seeking away whilst the
delta was being read.

This PR simply reads the delta into a buffer.

Fix go-git#323

Signed-off-by: Andrew Thornton <art27@cantab.net>
@starkers
Copy link

starkers commented Jun 1, 2021

I just tested this in my project and it appears to have resolved the issue, thanks for the quick turn around @zeripath

EDIT:
Actually.. its truncated some large files for some reason.. I'll have to investigate further
image

Don't know why these files but ye.. they've been simply wrecked.. I'll try to find some time to investigate further tomorrow

image

@puerco
Copy link

puerco commented Jun 2, 2021

Hi @zeripath, thanks a lot for working on this fix! I'm writing on behalf of the Kubernetes Release Engineering team. It seems that the issue this PR fixes broke the Kubernetes release process and we are unable to cut new versions right now 🙃

I was wondering If I can help you to merge this faster. Do you need help testing the patch or can we assist in any other way from our side? Thanks!!

@zeripath
Copy link
Contributor Author

zeripath commented Jun 2, 2021

@starkers agh. Ok so the packfile must still be being used elsewhere. I guess we need to duplicate the packfile fd for each of these. I'll take a look. Would you be willing to help test this functionality if I adjust this PR so that it switches this off by default?

@puerco - sorry about this. I'll adjust this pr to switch this off by default until we're sure it's working.

@hiddeco
Copy link
Member

hiddeco commented Jun 2, 2021

@zeripath we have been testing this PR in fluxcd/image-automation-controller#173 in the hope it would also solve another issue we have been running into around empty git-upload-pack given errors that are thrown since v5.4.0 (#328).

Is it possible there was another bug introduced somewhere that in some conditions sends empty packages? As this patch does not seem to have an effect.

@mcuadros
Copy link
Member

mcuadros commented Jun 2, 2021

I just created a new release with #329: https://github.com/go-git/go-git/releases/tag/v5.4.2

@zeripath The feature is still useful so feel free to commit it again, once the problems are solved

@mcuadros mcuadros closed this Jun 2, 2021
@zeripath
Copy link
Contributor Author

zeripath commented Jun 2, 2021

@mcuadros I'm deeply sorry about this.

Yeah we do still need to do something about the large files - especially to do with stating of files causing these files to end up memory.

I'll have another go at this. @starkers do you have a simple test case that can be used to replicate the issues?

@puerco
Copy link

puerco commented Jun 2, 2021

Thank you very much @zeripath and everyone involved, I can confirm we are able to clone large repos again. I am attaching here the test program I used to diagnose the problem when it hit us. I hope it helps. Let me know if we can help further :)

test-clone.go.gz

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.

latest "prevent large objects from being read into memory" commit caused "object not found" error
5 participants