Nguyễn Thái Ngọc Duy [off-list ref] writes:
quoted hunk
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 417c830..c58a9cb 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2709,6 +2709,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (get_oid_hex(skip_hash_hex, &skip_hash))
die(_("%s is not SHA-1"), skip_hash_hex);
}
+
+ /*
+ * Parallel delta search can't produce stable packs.
+ */
+ delta_search_threads = 1;
}
argv_array_push(&rp, "pack-objects");
A multi-threaded packing is _one_ source of regenerating the same
pack for the same set of objects, but we shouldn't be tying our
hands by promising it will forever be the _only_ source of it by
doing things like this. We may want to dynamically tweak the
packing behaviour depending on the load of the minute and such for
example.
This is an indication that the approach this series takes is taking
us in a wrong direction.
I think a more sensible approach for "resuming" is to attack cloning
first. Take a reasonable baseline snapshot periodically (depending
on the activity level of the project, the interval may span between
12 hours to 2 weeks and you would want to make it configurable) to
create a bundle, teach "clone" to check the bundle first and perform
a resumable and bulk transfer for the stable part of the history
(e.g. up to the latest tag or a branch that does not rewind, the set
of refs to use as the stable anchors you would want to make
configurable), and then fill the gap between that baseline snapshot
and up-to-date state by doing another round of "git fetch" and then
you'd have solved the most serious problem already.
On Sat, Feb 6, 2016 at 1:43 AM, Junio C Hamano [off-list ref] wrote:
Nguyễn Thái Ngọc Duy [off-list ref] writes:
quoted
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 417c830..c58a9cb 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2709,6 +2709,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (get_oid_hex(skip_hash_hex, &skip_hash))
die(_("%s is not SHA-1"), skip_hash_hex);
}
+
+ /*
+ * Parallel delta search can't produce stable packs.
+ */
+ delta_search_threads = 1;
}
argv_array_push(&rp, "pack-objects");
A multi-threaded packing is _one_ source of regenerating the same
pack for the same set of objects, but we shouldn't be tying our
hands by promising it will forever be the _only_ source of it by
doing things like this. We may want to dynamically tweak the
packing behaviour depending on the load of the minute and such for
example.
You noticed that tying the behavior only happens when the user asks
for it, right? I don't expect people to do resumable fetch/clone by
default. There are tradeoffs to make and they decide it, we offer
options. So, it does not really tie our hands in the normal case.
I think a more sensible approach for "resuming" is to attack cloning
first. Take a reasonable baseline snapshot periodically (depending
on the activity level of the project, the interval may span between
12 hours to 2 weeks and you would want to make it configurable) to
create a bundle, teach "clone" to check the bundle first and perform
a resumable and bulk transfer for the stable part of the history
(e.g. up to the latest tag or a branch that does not rewind, the set
of refs to use as the stable anchors you would want to make
configurable), and then fill the gap between that baseline snapshot
and up-to-date state by doing another round of "git fetch" and then
you'd have solved the most serious problem already.
_most_. On a troubled connection, fetch can fail as well, especially
when the repo is not uptodate. What about shallow clone? I don't think
you want to prepare snapshots for all depths (or some "popular"
depths). Cloning full then cutting it shallow locally might work, but
we're wasting bandwidth and disk space.
This is an indication that the approach this series takes is taking
us in a wrong direction.
The way I see it, the two approaches complement each other. Snapshots,
like pack bitmaps, helps tremendously, but it has corner cases that
can be covered by this.
--
Duy
You noticed that tying the behavior only happens when the user asks
for it, right? I don't expect people to do resumable fetch/clone by
default. There are tradeoffs to make and they decide it, we offer
options. So, it does not really tie our hands in the normal case.
You misread me. I do not want to see us having to add
"disable_this_feature = 1" next to that "delta_search_thread = 1"
in this block, and then supporting code to actually disable that
feature, only to support this block. You are declaring that "to
support this mode, we must always have a way to produce a
byte-for-byte identical output and keep supporting it forever".
On Sat, Feb 6, 2016 at 7:48 AM, Junio C Hamano [off-list ref] wrote:
quoted
You noticed that tying the behavior only happens when the user asks
for it, right? I don't expect people to do resumable fetch/clone by
default. There are tradeoffs to make and they decide it, we offer
options. So, it does not really tie our hands in the normal case.
You misread me. I do not want to see us having to add
"disable_this_feature = 1" next to that "delta_search_thread = 1"
in this block, and then supporting code to actually disable that
feature, only to support this block. You are declaring that "to
support this mode, we must always have a way to produce a
byte-for-byte identical output and keep supporting it forever".
My last defense is, this is an extension, not part of the core
protocol. If the burden becomes real we can always remove it.
Modem-quality connection users may yell a bit, but the majority of
broadband users won't notice a thing. But I understand if the answer
is still 'no'.
I find it unlikely that this cause much maintenance burden though, the
packing algorithm has not changed for a very long time (most related
change is pack bitmaps, which technically happen before pack-objects).
The next big change (at least in public) may be pack v4. We haven't
found a good algorithm for pack-objects on v4 yet, so there's a chance
of problems there.
--
Duy