Re: [PATCH] Prevent megablobs from gunking up git packs

"Dana How" [off-list ref] writes:

The packed X too big combination is the problem.  As the
commit message says,  this could happen if the packs
came from fast-import,...
We have three options in this case:
(1) Drop the object (do not put it in the new pack(s)).
(2) Pass the object into the new pack(s).
(3) Write out the object as a new loose object.

Option (1) is unacceptable.  When you call git-repack -a,
it blindly deletes all the non-kept packs at the end.  So
the megablobs would be lost.

Ok, I can buy that -- (1) nor (2) are unacceptable and (3) is
the only sane thing to do for a previously packed objects that
exceed the size limit.

Since you have to handle that case _anyway_, I think it makes
sense to always say "Ok, we will write it out if there is no
loose representation already available".

That is, unless somebody smarter than me, like Nico or Shawn,
come up with better ideas to do this ;-).

... why did I implement --max-blob-size instead
of --max-object-size?  I take this to mean that I should use
the blob size if undeltified, and the delta size if previously deltified?

No, I think the only sensible way for the end user to specify
the size is uncompressed size of the object.  For a blob, that
is the size of checked-out file.  IOW:

	$ git cat-file $type $sha | wc -c

Nothing else would make any sense.

Junio C Hamano [off-list ref] wrote:

"Dana How" [off-list ref] writes:

quoted

The packed X too big combination is the problem.  As the
commit message says,  this could happen if the packs
came from fast-import,...
We have three options in this case:
(1) Drop the object (do not put it in the new pack(s)).
(2) Pass the object into the new pack(s).
(3) Write out the object as a new loose object.

Option (1) is unacceptable.  When you call git-repack -a,
it blindly deletes all the non-kept packs at the end.  So
the megablobs would be lost.

Ok, I can buy that -- (1) nor (2) are unacceptable and (3) is
the only sane thing to do for a previously packed objects that
exceed the size limit.

I still don't buy the idea that these megablobs shouldn't be packed.
I understand Dana's pain here (at least a little bit, my problems
aren't as bad as his are), but I also hate to see us run away from
packfiles for these really sick cases just because we have some
issues in our current packfile handling.

Packfiles give us a lot of benefits:

 1) less inode usage;
 2) transport can write directly to local disk;
 3) transport can (quickly) copy from local disk;
 4) testing for existance is *much* faster;
 5) deltafication is possible;

Now #3 is actually really important here.  Don't forget that we
*just* disabled the fancy "new loose object format".  It doesn't
exist.  We can read the packfile-like loose objects, but we cannot
write them anymore.  So lets say we explode a megablob into a loose
object, and its 800 MiB by itself.  Now we have to send that object
to a client.  Yes, that's right, we must *RECOMPRESS* 800 MiB for
no reason.  Not the best choice.  Maybe we shouldn't have deleted
that packfile formatted loose object writer...

Now one argument to work around that recompression problem would
be to NFS share out the loose objects directory, and let clients
mount that volume and add it to their .git/objects/info/alternates
list.  But this doesn't work in the very general distributed case,
such as me getting huge files from kernel.org.  Last I checked,
the kernel.org admins did not offer up NSF mounts.  Besides, the
round-trip latency between me and kernel.org is too large for it
to be useful anyway over NFS.  :)

So I think this "explode out megablobs" is a bad idea.  Its violating
other things that make us fast, like #3's being able to reuse large
parts of an existing packfile during transfer.


Dana pointed out the megablobs make access slower because their
packfile indexes must still be searched to locate a commit; but if
the megablob packfile(s) contain only blobs then there is no value
in looking at those packfiles.

We might be able to fix this by altering the sort_pack function
in sha1_file.c to not only order by mtime, but also by the ratio
of the size of the .pack to the number of objects stored in it.
Any packfile with a high size/object ratio is likely to be what
Dana has been calling a "metadata" pack, holding things like tags,
commits, trees and small blobs.  Its these packfiles that we want
to search first, as they are the most likely to be accessed.

By pushing the megablob packs to the end of our packed_git search
list we won't tend to scan their indexes, as most of our objects
will be found earlier in the search list.  Hence we will generally
avoid any costs associated with their indexes.


Huge packfiles probably should be scheduled for keeping with a .keep
automatically.  We probably should teach pack-objects to generate a
.keep file if the resulting .pack was over a certain size threshold
(say 1.5 GiB by default) and teach git-repack to rename the .keep
file as it also renames the .idx and .pack.

Better that we degrade gracefully when faced with massive inputs
than we do something stupid by default and make the poor user pay
for their mistake of not throughly reading plumbing documentation
before use.


Now I would agree that we should punt on deltification of anything
that is just too large, and let the user decide what too large means,
and default it around 500 or 1024 MiB.  But I would still stuff it
into a packfile.

Maybe it still makes sense to have a limit on the maximum size of a
loose object to pack, but I think that's only to avoid the sick case
of a very simple no-argument "git repack" taking a long while because
there's 8 loose objects and 6 of them are 900 MiB image files.

Once in a packfile, I'd keep it there, even if the user decreases
the threshold, as the advantages of it being in the packfile outweigh
the disadvantages of it being in the packfile.  And there's like no
advantage to being loose once packed.


All of that is actually a very minor set of changes to the system,
and doesn't create odd corner cases.  It should also degrade better
out of the box.

quoted

... why did I implement --max-blob-size instead
of --max-object-size?  I take this to mean that I should use
the blob size if undeltified, and the delta size if previously deltified?

No, I think the only sensible way for the end user to specify
the size is uncompressed size of the object.  For a blob, that
is the size of checked-out file.  IOW:

	$ git cat-file $type $sha | wc -c

Nothing else would make any sense.

I agree.  And when you combine it with what I'm saying above about
only applying this to loose objects, its really quite easy to fetch
that value from the header and perform the test.

-- 
Shawn.

Hi,

On Thu, 24 May 2007, Shawn O. Pearce wrote:

Junio C Hamano [off-list ref] wrote:

quoted

"Dana How" [off-list ref] writes:

quoted

The packed X too big combination is the problem.  As the
commit message says,  this could happen if the packs
came from fast-import,...
We have three options in this case:
(1) Drop the object (do not put it in the new pack(s)).
(2) Pass the object into the new pack(s).
(3) Write out the object as a new loose object.

Option (1) is unacceptable.  When you call git-repack -a,
it blindly deletes all the non-kept packs at the end.  So
the megablobs would be lost.

Ok, I can buy that -- (1) nor (2) are unacceptable and (3) is
the only sane thing to do for a previously packed objects that
exceed the size limit.

I still don't buy the idea that these megablobs shouldn't be packed.
I understand Dana's pain here (at least a little bit, my problems
aren't as bad as his are), but I also hate to see us run away from
packfiles for these really sick cases just because we have some
issues in our current packfile handling.

Isn't this issue helpable by the "-delta" attribute?

Ciao,
Dscho

On Thu, 24 May 2007, Shawn O. Pearce wrote:

Now #3 is actually really important here.  Don't forget that we
*just* disabled the fancy "new loose object format".  It doesn't
exist.  We can read the packfile-like loose objects, but we cannot
write them anymore.  So lets say we explode a megablob into a loose
object, and its 800 MiB by itself.  Now we have to send that object
to a client.  Yes, that's right, we must *RECOMPRESS* 800 MiB for
no reason.  Not the best choice.  Maybe we shouldn't have deleted
that packfile formatted loose object writer...

when did the object store get changed so that loose objects aren't 
compressed?

if the problem is that the codepath for fetching does an uncompress 
followed by a compress then it would seem that this is a fairly easy 
problem to fix (how hard would it be to add the headers around the 
compressed object to make it look to the receiver like it's a pack with 
only one thing in it)

David Lang

Hi,

On Thu, 24 May 2007, david@lang.hm wrote:

On Thu, 24 May 2007, Shawn O. Pearce wrote:

quoted

Now #3 is actually really important here.  Don't forget that we
*just* disabled the fancy "new loose object format".  It doesn't
exist.  We can read the packfile-like loose objects, but we cannot
write them anymore.  So lets say we explode a megablob into a loose
object, and its 800 MiB by itself.  Now we have to send that object
to a client.  Yes, that's right, we must *RECOMPRESS* 800 MiB for
no reason.  Not the best choice.  Maybe we shouldn't have deleted
that packfile formatted loose object writer...

when did the object store get changed so that loose objects aren't
compressed?

That never happened. But we had a different file format for loose objects, 
which was meant to make it easier to copy as-is into a pack. That file 
format went away, since it was not as useful as we hoped.

Ciao,
Dscho

[resent because of malformed headers causing rejection]
On May 24, 2007, at 03:12, Shawn O. Pearce wrote:

I still don't buy the idea that these megablobs shouldn't be packed.
I understand Dana's pain here (at least a little bit, my problems
aren't as bad as his are), but I also hate to see us run away from
packfiles for these really sick cases just because we have some
issues in our current packfile handling.

Packfiles give us a lot of benefits:

 1) less inode usage;

Using 1 inode per huge blob can never be an issue

 2) transport can write directly to local disk;
 3) transport can (quickly) copy from local disk;

Can do these by re-enabling the new loose object format

 4) testing for existance is *much* faster;
 5) deltafication is possible;

Look at it the other way. If we have huge objects (say >1GB),
we should put them in a pack of their own anyway. What's better:
having a pack with a separate index file or just a loose object?
While the one object per file model is awful for many small files
with lots of similarity, it is really quite efficient for large
objects, and the most reasonable model for huge objects.

Such blobs are just too large to do anything useful with.
The only operations done on them will be to check them in
or check them out. Ideally, we should never try to have them
in memory at all, but just stream them to/from disk while
compressing or decompressing.

Trying to deltify huge objects just takes too much time.
Similarly, we don't want to read 100MB to then apply a delta
and maybe throw out half of the data we read in the first place.
It's just too inefficient. If we'd even read the huge blobs once
during "git repack", we'll waste so much time that we're unlikely
to ever gain it back in any real world scenario.

   -Geert

On 5/24/07, Shawn O. Pearce [off-list ref] wrote:

Junio C Hamano [off-list ref] wrote:

quoted

"Dana How" [off-list ref] writes:

quoted

We have three options in this case:
(1) Drop the object (do not put it in the new pack(s)).
(2) Pass the object into the new pack(s).
(3) Write out the object as a new loose object.
Option (1) is unacceptable.  When you call git-repack -a,
it blindly deletes all the non-kept packs at the end.  So
the megablobs would be lost.

Ok, I can buy that -- (1) nor (2) are unacceptable and (3) is
the only sane thing to do for a previously packed objects that
exceed the size limit.

I still don't buy the idea that these megablobs shouldn't be packed.
I understand Dana's pain here (at least a little bit, my problems
aren't as bad as his are), but I also hate to see us run away from
packfiles for these really sick cases just because we have some
issues in our current packfile handling.

Packfiles give us a lot of benefits:

 1) less inode usage;

I agree with Geert that blowing an inode on a 100MB+ object
is no big deal.

 2) transport can write directly to local disk;
 3) transport can (quickly) copy from local disk;

For (2) and (3) see comments on next para plus NFS discussion.

 4) testing for existance is *much* faster;

This is true.  But I don't care about this cost if it
is only incurred on large objects which are leaf nodes
in the git "data relationship tree" (tags->commits->trees->blobs) anyway.

 5) deltafication is possible;

Again Geert made a good argument that didn't occur to me that
you definitely DON'T want to do deltification on such large objects.
Junio recently added delta/nodelta attribute; this would be useful
to me,  but unfortunately I have several continua of files,  each with
the same suffix,  but with largely varying sizes, so attributes won't
help me unless the name globs in .gitattributes are expanded to full
expressions similar to find(1) [i.e. include testing based on size,
perms, type],  which I think would be insane.

Now #3 is actually really important here.  Don't forget that we
*just* disabled the fancy "new loose object format".  It doesn't
exist.  We can read the packfile-like loose objects, but we cannot
write them anymore.  So lets say we explode a megablob into a loose
object, and its 800 MiB by itself.  Now we have to send that object
to a client.  Yes, that's right, we must *RECOMPRESS* 800 MiB for
no reason.  Not the best choice.  Maybe we shouldn't have deleted
that packfile formatted loose object writer...

I completely agree with your argument.  I do not suggest that repositories
that communicate with others via packs should use this feature.
Our repositories will communicate via alternates/NFS in one direction
and probably packs in the other.  In the latter case the packs would
be generated with maxblobsize=0. See comments on next para.

Now one argument to work around that recompression problem would
be to NFS share out the loose objects directory, and let clients
mount that volume and add it to their .git/objects/info/alternates
list.  But this doesn't work in the very general distributed case,
such as me getting huge files from kernel.org.  Last I checked,
the kernel.org admins did not offer up NSF mounts.  Besides, the
round-trip latency between me and kernel.org is too large for it
to be useful anyway over NFS.  :)

The NFS case is exactly what I want to use.  I want each repo to
have their own packfiles to reduce load on the central alternate,
but these local repos would not include megablobs.  I do not have
as strong a feeling about whether the central alternate should
pack its megablobs or not [but I don't want to do it if it costs me
deltification for everybody], but I need a way to exclude megablobs
from getting into local packs.  WIth such exclusion,  git-gc/repack
is extremely quick.  There is NO WAY this can be true if
several GB have to be copied around,  which again comes from Geert.

I think this conversation does suggest one alteration to my patch:
as submitted it writes out a loose object if the object is packed
and has no loose object.  It should really do this only if the
object is packed LOCALLY and has no loose object.

So I think this "explode out megablobs" is a bad idea.  Its violating
other things that make us fast, like #3's being able to reuse large
parts of an existing packfile during transfer.

As I said,  your true argument doesn't apply in my case.

Dana pointed out the megablobs make access slower because their
packfile indexes must still be searched to locate a commit; but if
the megablob packfile(s) contain only blobs then there is no value
in looking at those packfiles.

We might be able to fix this by altering the sort_pack function
in sha1_file.c to not only order by mtime, but also by the ratio
of the size of the .pack to the number of objects stored in it.
Any packfile with a high size/object ratio is likely to be what
Dana has been calling a "metadata" pack, holding things like tags,
commits, trees and small blobs.  Its these packfiles that we want
to search first, as they are the most likely to be accessed.

By pushing the megablob packs to the end of our packed_git search
list we won't tend to scan their indexes, as most of our objects
will be found earlier in the search list.  Hence we will generally
avoid any costs associated with their indexes.

Good argument and I submitted a patch to do this.
Let's see who chokes on the floating arithmetic ;-)

Huge packfiles probably should be scheduled for keeping with a .keep
automatically.  We probably should teach pack-objects to generate a
.keep file if the resulting .pack was over a certain size threshold
(say 1.5 GiB by default) and teach git-repack to rename the .keep
file as it also renames the .idx and .pack.

I have experimented with this,  and Jakub Narebski made related
suggestions.  I find this quite hokey,  but even if I do it in my central
alternate,  I still do not want to be packing megablobs in individual user's
repos EVER,  and need some way to exclude them.

Better that we degrade gracefully when faced with massive inputs
than we do something stupid by default and make the poor user pay
for their mistake of not throughly reading plumbing documentation
before use.

Unnecessary copying of several GB is not degrading gracefully in my view.
In fact having repack.maxblobsize = 2000 (K) in the default "config"
strikes me as degrading much more gracefully than what the code
would currently do.

This silly patch took my packfile sets from 12GB+ to 13MB,
and it's difficult to describe how relieved I now feel.

It's also difficult for me to believe that a setup that treats 12GB
(almost) equally is going to be as efficient as one which concentrates on 13MB.
That's three orders of magnitude,  a point I've made before.

But I think you have an understandable motivation:
you want packfiles to be as good as possible,  and any escape
mechanism from them decreases the motivation to "fix" packing.
Now I agree with this, which is why I just submitted some other patches,
but I don't share your goal of the universality of all packfiles --
just the ones used for transport.  Don't your packv4 plans introduce
mods which won't be used for transport as well?

Now I would agree that we should punt on deltification of anything
that is just too large, and let the user decide what too large means,
and default it around 500 or 1024 MiB.  But I would still stuff it
into a packfile.

Maybe it still makes sense to have a limit on the maximum size of a
loose object to pack, but I think that's only to avoid the sick case
of a very simple no-argument "git repack" taking a long while because
there's 8 loose objects and 6 of them are 900 MiB image files.

Perhaps we _will_ make progress if we all agree to describe
my situation as "sick" ;-) .  In this paragraph you seem to agree that
there is some argument for keeping megablobs from _entering_ packs?

One reason I like my patch is because I do view megablobs
as perverting the system,  and just keeping them out of the optimized
packfile system is a big step forward.

Once in a packfile, I'd keep it there, even if the user decreases
the threshold, as the advantages of it being in the packfile outweigh
the disadvantages of it being in the packfile.  And there's like no
advantage to being loose once packed.

To (almost) follow this suggestion I would need git-fast-import to respect
repack.maxblobsize as well.  Is that OK with you?

I previously offered to Junio that the "write loose object" thing
could be restricted:  it would only happen if -f were supplied to
git-repack,  otherwise the bad blob would pass through to the new pack.
Does this "reduction in strength" make this feature more palatable to you?

If the stats on a repo change significantly,  "write loose object"
becomes more important if you have to make a significant reduction
to repack.maxblobsize (or specify it for the first time).

I don't agree that once in a packfile,  a blob should stay there.
Its presence is degrading access to "normal" blobs co-habiting with it.
So you will want to repack to separate them in different packs
(the various .keep-related ideas) or just write them out loose.

To conclude:
the patch wrote out a new loose object when it was previously
packed and is larger then repack.maxblobsize.
I could change this to only happen when the object is
(1) packed AND
(2) locally packed AND
(3) -f/--no-object-reuse was specified to git-repack/git-pack-objects.
The previous behavior that a megablob never _enters_ the pack
would remain unchanged.

This more restrictive behavior would be sufficient for me,
and I think I *need* it at least in the users' repositories
in an NFS/alternates setup.

What do you think?

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Johannes Schindelin [off-list ref] wrote:

On Thu, 24 May 2007, david@lang.hm wrote:

quoted

On Thu, 24 May 2007, Shawn O. Pearce wrote:

quoted

Now #3 is actually really important here.  Don't forget that we
*just* disabled the fancy "new loose object format".  It doesn't
exist.  We can read the packfile-like loose objects, but we cannot
write them anymore.  So lets say we explode a megablob into a loose
object, and its 800 MiB by itself.  Now we have to send that object
to a client.  Yes, that's right, we must *RECOMPRESS* 800 MiB for
no reason.  Not the best choice.  Maybe we shouldn't have deleted
that packfile formatted loose object writer...

when did the object store get changed so that loose objects aren't
compressed?

That never happened. But we had a different file format for loose objects, 
which was meant to make it easier to copy as-is into a pack. That file 
format went away, since it was not as useful as we hoped.

That "different file format" thing was added exactly for this type
of problem.  Someone added a bunch of large blobs to their repository
and then spent a lot of time decompressing and recompressing them
during their next repack.

The reason that recompress must happen is the deflate stream in a
standard (aka legacy) loose object contains both the Git object
header and the raw data; in a packfile the Git object header is
stored external from the deflate stream.  The "different file format"
used the packfile format, allowing us to store the Git object header
external from the deflate stream.  That meant we could just copy
the raw bytes as-is from the loose object into the packfile.

So we still store loose objects compressed, its just that we can
no longer create loose objects that can be copied directly into
a packfile without recompression.  And that is sort of Dana's
problem here.  OK, not entirely, but whatever.

-- 
Shawn.

Dana How [off-list ref] wrote:

On 5/24/07, Shawn O. Pearce [off-list ref] wrote:

quoted

Junio C Hamano [off-list ref] wrote:

quoted

"Dana How" [off-list ref] writes:

quoted

We have three options in this case:
(1) Drop the object (do not put it in the new pack(s)).
(2) Pass the object into the new pack(s).
(3) Write out the object as a new loose object.
Option (1) is unacceptable.  When you call git-repack -a,
it blindly deletes all the non-kept packs at the end.  So
the megablobs would be lost.

Ok, I can buy that -- (1) nor (2) are unacceptable and (3) is
the only sane thing to do for a previously packed objects that
exceed the size limit.

I still don't buy the idea that these megablobs shouldn't be packed.
I understand Dana's pain here (at least a little bit, my problems
aren't as bad as his are), but I also hate to see us run away from
packfiles for these really sick cases just because we have some
issues in our current packfile handling.

Packfiles give us a lot of benefits:

1) less inode usage;

I agree with Geert that blowing an inode on a 100MB+ object
is no big deal.

You and me both.  If the size of the blob is high enough than
overhead associated with the inode and any tail-block wastage is
noise.  Filesystems are pretty good at tracking large-ish files.
My comment here wasn't so much about blowing an inode on a 100MiB+
object, but just in general that packfiles reduce inode usage,
which in the common metadata case (300,000 small objects) is a
big difference.

I think you are right; if we get an object in the >100MiB size
range we can certainly afford an inode for it.

quoted

2) transport can write directly to local disk;
3) transport can (quickly) copy from local disk;

For (2) and (3) see comments on next para plus NFS discussion.

quoted

4) testing for existance is *much* faster;

This is true.  But I don't care about this cost if it
is only incurred on large objects which are leaf nodes
in the git "data relationship tree" (tags->commits->trees->blobs) anyway.

Yes, that's true.

quoted

5) deltafication is possible;

Again Geert made a good argument that didn't occur to me that
you definitely DON'T want to do deltification on such large objects.
Junio recently added delta/nodelta attribute; this would be useful
to me,  but unfortunately I have several continua of files,  each with
the same suffix,  but with largely varying sizes, so attributes won't
help me unless the name globs in .gitattributes are expanded to full
expressions similar to find(1) [i.e. include testing based on size,
perms, type],  which I think would be insane.

Which brings up the comment I think I made (below) about skipping
deltas on very large objects.  Things over a certain size are not
likely to delta well, or in any reasonable time.  We probably should
default to not trying to delta those, but let the user force us to
do so with a .gitattributes option.  Maybe.

quoted

By pushing the megablob packs to the end of our packed_git search
list we won't tend to scan their indexes, as most of our objects
will be found earlier in the search list.  Hence we will generally
avoid any costs associated with their indexes.

Good argument and I submitted a patch to do this.
Let's see who chokes on the floating arithmetic ;-)

I actually had another thought in this area.  I'll try to work up a
patch to accompany yours.  I think we can avoid even touching the
alternate object databases half of the time, and I'd like to be
able to do that.  Why?  Because I started to setup this megablob
approach on my own Windows based repositories.  Unfortunately it
makes git-log about 1 second slower, and I suspect its in the
alternate repository initialization.
 

quoted

Huge packfiles probably should be scheduled for keeping with a .keep
automatically.  We probably should teach pack-objects to generate a
.keep file if the resulting .pack was over a certain size threshold
(say 1.5 GiB by default) and teach git-repack to rename the .keep
file as it also renames the .idx and .pack.

I have experimented with this,  and Jakub Narebski made related
suggestions.  I find this quite hokey,  but even if I do it in my central
alternate,  I still do not want to be packing megablobs in individual user's
repos EVER,  and need some way to exclude them.

Yes, that makes a lot of sense.

quoted

Better that we degrade gracefully when faced with massive inputs
than we do something stupid by default and make the poor user pay
for their mistake of not throughly reading plumbing documentation
before use.

Unnecessary copying of several GB is not degrading gracefully in my view.
In fact having repack.maxblobsize = 2000 (K) in the default "config"
strikes me as degrading much more gracefully than what the code
would currently do.

Sure.  But I think this goes back to #3 (network transport) and
how our loose object format now doesn't support it well.  And even
if that's fixed I don't think 2 MiB is a good default; its *far*
too low.  I have a number of blobs that are in the 12-16 MiB range
and they delta very well in a pretty reasonable time.

This silly patch took my packfile sets from 12GB+ to 13MB,
and it's difficult to describe how relieved I now feel.

I think I understand a little bit.  Today I took 3 repositories
that were about 70 MiB each and dropped them down to 16 MiB, 2
MiB and 4.5 MiB by creating a single 120 MiB "megablob" packfile
that spanned all 3 of them.  This isn't the same scale as what you
are dealing with, but now I have a current metadata pack for each
that isn't gummed up with large blobs, making repacking faster.
I also have a smaller working set size.  :-

But I think you have an understandable motivation:
you want packfiles to be as good as possible,  and any escape
mechanism from them decreases the motivation to "fix" packing.

Yes, that's correct.  I'm not against stepping outside of
packfiles and making usage of loose objects for megablobs easier.
I just want to make sure its the best way to handle these things.
Generally we've made major improvements in things when we've been
pushed by large repositories/datasets.

Now I agree with this, which is why I just submitted some other patches,
but I don't share your goal of the universality of all packfiles --
just the ones used for transport.  Don't your packv4 plans introduce
mods which won't be used for transport as well?

Yes, at least initially we'd reencode from pack v4 down to pack v2
for transport, because transporting the dictionary with delta reuse
is an interesting problem.  However Nico and I have discussed it
at length and have plans for how to code a pack v4 based transport,
and pack v4's file format concepts are partially based upon making
pack v4 transport easier to implement.  But from a "start simple and
keep it simple, stupid" principle we'd like to avoid the complexity
early on.

quoted

Now I would agree that we should punt on deltification of anything
that is just too large, and let the user decide what too large means,
and default it around 500 or 1024 MiB.  But I would still stuff it
into a packfile.

Maybe it still makes sense to have a limit on the maximum size of a
loose object to pack, but I think that's only to avoid the sick case
of a very simple no-argument "git repack" taking a long while because
there's 8 loose objects and 6 of them are 900 MiB image files.

Perhaps we _will_ make progress if we all agree to describe
my situation as "sick" ;-) .  In this paragraph you seem to agree that
there is some argument for keeping megablobs from _entering_ packs?

Yes.  If your workflow is basically "git add HUGE; git commit; git push;
git prune-alternates" then you only have to pack the huge object once,
and can remove the loose object from the user's .git/objects directly
pretty quickly, because its available via your NFS alternate.  In such
a configuration yes, it does make some sense to never allow a megablob
from entering a pack.

So I guess I'm partially in agreement with you...

quoted

Once in a packfile, I'd keep it there, even if the user decreases
the threshold, as the advantages of it being in the packfile outweigh
the disadvantages of it being in the packfile.  And there's like no
advantage to being loose once packed.

To (almost) follow this suggestion I would need git-fast-import to respect
repack.maxblobsize as well.  Is that OK with you?

Yes I could implement that (or better accept a patch that does so)
but I'd actually wonder why not just categorize the objects into
two different packfiles.  Have one for "small stuff" and another
for "everything larger than small stuff".  Split the two packfiles
independently of each other.  Hence fast-import would produce more
packfiles, but each output packfile would probably have a couple
of megablobs in it, and you'd have one single packfile with all of
the smaller metadata.

And actually if you are trying to shove large objects through
fast-import we really can do a lot better.  Like avoiding
deltification attempts, adjusting the compression level to something
better suited to your blob (I don't know if its compressable or not)
and streaming to the output packfile, rather than holding the entire
thing in memory before writing the first byte.

Given the advantages discussed above about being in a packfile, and
that fast-import was writing specifically for creating packfiles
instead of loose objects during large IO transfers into Git, I
think it is sort of anti-fast-import to have it create loose objects.
 

I previously offered to Junio that the "write loose object" thing
could be restricted:  it would only happen if -f were supplied to
git-repack,  otherwise the bad blob would pass through to the new pack.
Does this "reduction in strength" make this feature more palatable to you?

Yes.  But go back to the .keep discussion above where I suggest
we automatically .keep any "large" packfile.  Once you get a huge
packfile you probably don't want to redo the disk IO associated with
repacking it, unless you really are trying to force a large reorg.
So I'd agree with the idea of making a -f needed to eject a megablob,
but I think you'd need more than that as you'd also be trying to
bypass the usual .keep logic.
 

If the stats on a repo change significantly,  "write loose object"
becomes more important if you have to make a significant reduction
to repack.maxblobsize (or specify it for the first time).

I'm not sure users should be tweaking this...  Its fine to make
the knob available, but we really should have the knob adjust
itself somewhat intelligently.  Don't ever add a knob that you
cannot write a rule to control; if you can write a rule than write
it dammit and don't make the user do your job for you.  Knobs are
good for when your case is just so far away from the normal that
the rule is utterly wrong...

Guess what, your repository is such a repository (its far away from
our normal rules).  But I think its problems are also common enough
that we really should attempt to make our rules handle it better.
 

I don't agree that once in a packfile,  a blob should stay there.
Its presence is degrading access to "normal" blobs co-habiting with it.
So you will want to repack to separate them in different packs
(the various .keep-related ideas) or just write them out loose.

If we're evicting a megablob from one packfile to a loose object
(because its degrading access to the other objects in its current
packfile) we're already committed to doing the massive disk IO
required for that eviction.  We might as well write it back out
to a file format that we can more easily work with, than one that
we cannot.

But I have to circle back here and say "why is a megablob degrading
access in a packfile"?  This goes right back to your point above
about my wanting to stay in the packfile format just to make the
packfile format better.  What's so wrong with the packfile format
(and the code that writes/reads it) that makes access for a small
metadata more expensive when there are megablobs attached in the
same packfile?

Or is it just because we like to repack the smaller metadata
frequently, but that's horribly expensive because the megablobs
are in the same packfile?  If its really just about repacking then
.keep marked megablob packs are the way to go.

-- 
Shawn.

OK..... I ignore git@vger.kernel.org for a day or two and things really 
start to go wild!  ;-)

I'll try to cover only those points that are still debatable.  I think 
everybody agrees with huge blobs as loose objects using extra inodes 
being the least of our worries.

On Thu, 24 May 2007, Shawn O. Pearce wrote:

Dana How [off-list ref] wrote:

quoted

On 5/24/07, Shawn O. Pearce [off-list ref] wrote:

quoted

Junio C Hamano [off-list ref] wrote:

quoted

"Dana How" [off-list ref] writes:

quoted

We have three options in this case:
(1) Drop the object (do not put it in the new pack(s)).
(2) Pass the object into the new pack(s).
(3) Write out the object as a new loose object.
Option (1) is unacceptable.  When you call git-repack -a,
it blindly deletes all the non-kept packs at the end.  So
the megablobs would be lost.

Ok, I can buy that -- (1) nor (2) are unacceptable and (3) is
the only sane thing to do for a previously packed objects that
exceed the size limit.

OK... I sort of agree, but not entirely.

First, let's examine the reasons for wanting to expulse a big blob out 
of a pack.

The first reason I've seen is that big blobs put surrounding objects way 
apart and pack access performance gets bad, especially tree walking.  
The solution to this problem is trivial: let's simply store big blobs 
together at the end of the pack!  Problem solved.

The other reason for keeping huge blobs out is that they bring repack 
performance down and create unnecessary IO.  Well, in that case I think 
that you should simply avoid (re)packing them in the first place.  I 
think it should be possible to combine both features: the split packs 
and the big-blobs-go-at-the-end solution I mentioned above so that those 
big blobs could end up in one or more packs of their own.

But writing loose objects from git-pack-objects... Nah, this is just too 
hacky and ugly.  The tool is about packing objects and starting to 
create loose objects from there is pushing the packing concept a bit too 
far for my taste.

I wouldn't mind a _separate_ tool that would load a pack index, 
determine object sizes from it, and then extract big objects to write 
them as loose objects (although I question its usefulness).  But not 
within pack-objects please.

So I think the best solution really involves a new parameter to 
git-pack-objects allowing for objects which size exceed a certain 
treshold to go at the end of the pack.  If they end up in a different 
pack because of pack size limit then so be it, at which point you could 
always explode that huge-blob pack into loose objects, avoiding the need 
for the extra tool I mention above, but again I don't think that would 
be that useful.

quoted

Again Geert made a good argument that didn't occur to me that
you definitely DON'T want to do deltification on such large objects.
Junio recently added delta/nodelta attribute; this would be useful
to me,  but unfortunately I have several continua of files,  each with
the same suffix,  but with largely varying sizes, so attributes won't
help me unless the name globs in .gitattributes are expanded to full
expressions similar to find(1) [i.e. include testing based on size,
perms, type],  which I think would be insane.

I think having a parameter to exclude object which size exceed a 
specified size treshold from deltification attempts would also be a 
valid option. But...

Which brings up the comment I think I made (below) about skipping
deltas on very large objects.  Things over a certain size are not
likely to delta well, or in any reasonable time.  We probably should
default to not trying to delta those, but let the user force us to
do so with a .gitattributes option.  Maybe.

I don't agree with the presumption that huge objects are unlikely to 
delta well.  It really depends on the data you have.  If, for example, 
you want to store, say, different versions of a filesystem image, then 
those different images have the potential to be really huge. Yet they 
might delta extremely well against each other and provide a tremendous 
space saving.

It all depends on the kind of data you work with.
It is good to have the possibility to skip deltification based on a file 
attribute.  It is also good to have the possibility to skip 
deltification based on object size (through a command line switch or 
config entry).  But those must remain _options_.

quoted

quoted

Huge packfiles probably should be scheduled for keeping with a .keep
automatically.  We probably should teach pack-objects to generate a
.keep file if the resulting .pack was over a certain size threshold
(say 1.5 GiB by default) and teach git-repack to rename the .keep
file as it also renames the .idx and .pack.

Nah.  Those kind of arbitrary defaults are most likely to be fine for 
some cases and bad for many others.  These "sick" cases such as Dana's 
are so special that they better be manually tuned for best operations 
according to the data set, and more importantly to the work flow used, 
because different work flows are likely to require different "defaults".  
Better not put any arbitrary default and create a "Advanced tuning for 
best performances with insane repositories" section in the documentation 
instead.

quoted

quoted

Better that we degrade gracefully when faced with massive inputs
than we do something stupid by default and make the poor user pay
for their mistake of not throughly reading plumbing documentation
before use.

Well, I think that if someone is seriously considering GIT for a 
multi-gigabyte repository, that person has better read a little 
documentation before starting to play.  Of course this advanced tuning 
for huge repository section I'm suggesting should stand out in the main 
index.  And most "poor users" usually don't have such a big repo to 
fool themselves with.

quoted

quoted

Now I would agree that we should punt on deltification of anything
that is just too large, and let the user decide what too large means,
and default it around 500 or 1024 MiB.  But I would still stuff it
into a packfile.

Well, thing is, once deltified, those huge objects won't be subject to 
deltification attempts anymore, unless -f is used.  So the deltification 
cost will happen only once anyway.  Then it is only the issue of 
flagging a particular pack with .keep to exclude it from any further 
repacking which would simply end up wasting disk IO anyway.

There is certainly a hard default on deltification attempt that we 
should impose right now though, which is 4GB.  The reason is that the 
delta encoding doesn't do offsets larger than 32 bits at the moment.

quoted

I previously offered to Junio that the "write loose object" thing
could be restricted:  it would only happen if -f were supplied to
git-repack,  otherwise the bad blob would pass through to the new pack.
Does this "reduction in strength" make this feature more palatable to you?

Not really.

Like I said before, I'd much prefer to have a split pack for huge 
objects, and a separate unpack-object pass on it if you really want them 
loose.  If you want to deny entry of loose objects into a pack based on 
their size that's understandable, but only if they're already loose.

quoted

I don't agree that once in a packfile,  a blob should stay there.
Its presence is degrading access to "normal" blobs co-habiting with it.

As mentioned at the top I don't think this is a big issue.

Or is it just because we like to repack the smaller metadata
frequently, but that's horribly expensive because the megablobs
are in the same packfile?  If its really just about repacking then
.keep marked megablob packs are the way to go.

I think so as well.


Nicolas