Christian Couder [off-list ref] writes:

quoted

Minor nit.  Aren't the above two the same use case?

In the first case only some large blobs are moved to slower storage
and in the other case all the blobs are moved to slower storage. So
yeah the use cases are very similar. Not sure if and how I can improve
the above wording though.

Just by removing one or the other, it would be quite improved, no?
Moving away some blobs could move away all or just a selected
subset.

I think it can work if the call to write_filtered_pack() is either
before the call to close_object_store() or after it. It would just use
the tempfiles with their temporary name in the first case and with
their final name in the second case.

write_filtered_pack() is very similar to write_cruft_pack() which is
called before the call to close_object_store(), so I prefer to keep it
before that call too, if possible, for consistency.

As long as the set-up is not racy, either would be OK, as the names
are not recorded in the end result.

If the upstream tells the downstream the temporary's name and then
finializes the temporary to the final name before the downstream
reacts to the input, however, then by the time downstream starts
working on the file, the file may not exist under its original,
temporary name, and that kind of race was what I was worried about.

Perhaps this could be dealt with separately though, as I think we
might want to fix write_cruft_pack() first then.

Sorry, I am not understanding this quite well.  Do you mean we
should add one more known-to-be-racy-and-incorrect codepath because
there is already a codepath that needs to be fixed anyway?

If write_cruft_pack() has a similar issue, then yeah, let's fix that
first (testing might be tricky for any racy bugs).  And let's use
the same technique as used to fix it in this series, too, so that we
do not reintroduce a similar bug due to racy setup.

Thanks.