On Sat, Dec 03, 2016 at 04:24:02PM -0800, Kyle J. McKay wrote:
> When the incoming quarantine takes place the current objects directory
> is demoted to an alternate thereby increasing its depth (and any
> alternates it references) by one and causing any object store that was
> previously at the maximum nesting depth to be ignored courtesy of the
> above hard-coded maximum depth.
>
> If the incoming push happens to need access to some of those objects
> to perhaps "--fix-thin" its pack it will crash and burn.
Yep, that makes sense. I didn't really worry about this because the
existing "5" is totally arbitrary, and meant to be so high that nobody
reaches it (it's just there to break cycles).
So I do think this is worth dealing with, but I'm also curious why
you're hitting the depth-5 limit. I'm guessing it has to do with hosting
a hierarchy of related repos. But is your system then always in danger
of busting the 5-limit if people create too deep a repository hierarchy?
Specifically, I'm wondering if it would be sufficient to just bump it to
6. Or 100.
Of course any static bump runs into the funny case where a repo
_usually_ works, but fails when pushed to. Which is kind of nasty and
unintuitive. And your patch fixes that, and we can leave the idea of
bumping the static depth number as an orthogonal issue (that personally,
I do not care about much about either way).
> diff --git a/common-main.c b/common-main.c
> index c654f955..9f747491 100644
> --- a/common-main.c
> +++ b/common-main.c
> @@ -37,5 +37,8 @@ int main(int argc, const char **argv)
>
> restore_sigpipe_to_default();
>
> + if (getenv(GIT_QUARANTINE_ENVIRONMENT))
> + alt_odb_max_depth++;
> +
> return cmd_main(argc, argv);
After reading your problem description, my initial thought was to
increment the counter when we allocate the tmp-objdir, and decrement
when it is destroyed. Because the parent receive-pack process adds it to
its alternates, too. But:
1. Receive-pack doesn't care; it adds the tmp-objdir as an alternate,
rather than adding it as its main object dir and bumping down the
main one.
2. There would have to be some way of communicating to sub-processes
that they should bump their max-depth by one.
You've basically used the quarantine-path variable as the
inter-process flag for (2). Which feels a little funny, because its
value is unrelated to the alt-odb setup. But it is a reliable signal, so
there's a certain elegance. It's probably the best option, given that
the alternative is a specific variable to say "hey, bump your
max-alt-odb-depth by one". That's pretty ugly, too. :)
-Peff