git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: git@vger.kernel.org, John Cai <johncai86@gmail.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Taylor Blau <me@ttaylorr.com>, Derrick Stolee <stolee@gmail.com>,
	Patrick Steinhardt <ps@pks.im>,
	Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH v2 5/8] repack: add `--filter=<filter-spec>` option
Date: Mon, 24 Jul 2023 11:28:43 -0700	[thread overview]
Message-ID: <xmqqila9p3j8.fsf@gitster.g> (raw)
In-Reply-To: <CAP8UFD0aa+EZQ2Q=C2WjWrNL9desg-KLLjOKS8BUBR4DS1ytsQ@mail.gmail.com> (Christian Couder's message of "Mon, 24 Jul 2023 11:01:48 +0200")

Christian Couder <christian.couder@gmail.com> writes:

>> 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.


  reply	other threads:[~2023-07-24 18:29 UTC|newest]

Thread overview: 161+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-14 19:25 [PATCH 0/9] Repack objects into separate packfiles based on a filter Christian Couder
2023-06-14 19:25 ` [PATCH 1/9] pack-objects: allow `--filter` without `--stdout` Christian Couder
2023-06-21 10:49   ` Taylor Blau
2023-07-05  6:16     ` Christian Couder
2023-06-14 19:25 ` [PATCH 2/9] pack-objects: add `--print-filtered` to print omitted objects Christian Couder
2023-06-15 22:50   ` Junio C Hamano
2023-06-21 10:52     ` Taylor Blau
2023-06-21 11:11       ` Christian Couder
2023-06-21 11:54         ` Taylor Blau
2023-06-14 19:25 ` [PATCH 3/9] t/helper: add 'find-pack' test-tool Christian Couder
2023-06-15 23:32   ` Junio C Hamano
2023-06-21 10:40     ` Christian Couder
2023-06-21 10:54     ` Taylor Blau
2023-06-14 19:25 ` [PATCH 4/9] repack: refactor piping an oid to a command Christian Couder
2023-06-15 23:46   ` Junio C Hamano
2023-06-21 10:55     ` Taylor Blau
2023-06-21 10:56     ` Christian Couder
2023-06-14 19:25 ` [PATCH 5/9] repack: refactor finishing pack-objects command Christian Couder
2023-06-16  0:13   ` Junio C Hamano
2023-06-21 11:06     ` Taylor Blau
2023-06-21 11:19       ` Christian Couder
2023-06-21 11:05   ` Taylor Blau
2023-06-14 19:25 ` [PATCH 6/9] repack: add `--filter=<filter-spec>` option Christian Couder
2023-06-16  0:43   ` Junio C Hamano
2023-06-21 11:20     ` Taylor Blau
2023-06-21 15:04       ` Christian Couder
2023-06-22 11:05         ` Taylor Blau
2023-06-21 14:40     ` Christian Couder
2023-06-21 16:53       ` Junio C Hamano
2023-06-22  8:39         ` Christian Couder
2023-06-22 18:32           ` Junio C Hamano
2023-06-21 11:17   ` Taylor Blau
2023-07-05  7:18     ` Christian Couder
2023-06-14 19:25 ` [PATCH 7/9] gc: add `gc.repackFilter` config option Christian Couder
2023-06-14 19:25 ` [PATCH 8/9] repack: implement `--filter-to` for storing filtered out objects Christian Couder
2023-06-16  2:21   ` Junio C Hamano
2023-06-21 11:49   ` Taylor Blau
2023-06-21 12:08     ` Christian Couder
2023-06-21 12:25       ` Taylor Blau
2023-06-21 16:44         ` Junio C Hamano
2023-07-05  6:19     ` Christian Couder
2023-06-14 19:25 ` [PATCH 9/9] gc: add `gc.repackFilterTo` config option Christian Couder
2023-06-16  2:54   ` Junio C Hamano
2023-06-14 21:36 ` [PATCH 0/9] Repack objects into separate packfiles based on a filter Junio C Hamano
2023-06-16  3:08   ` Junio C Hamano
2023-07-05  6:08 ` [PATCH v2 0/8] " Christian Couder
2023-07-05  6:08   ` [PATCH v2 1/8] pack-objects: allow `--filter` without `--stdout` Christian Couder
2023-07-05  6:08   ` [PATCH v2 2/8] t/helper: add 'find-pack' test-tool Christian Couder
2023-07-05  6:08   ` [PATCH v2 3/8] repack: refactor finishing pack-objects command Christian Couder
2023-07-05  6:08   ` [PATCH v2 4/8] repack: refactor finding pack prefix Christian Couder
2023-07-05  6:08   ` [PATCH v2 5/8] repack: add `--filter=<filter-spec>` option Christian Couder
2023-07-05 17:53     ` Junio C Hamano
2023-07-24  9:01       ` Christian Couder
2023-07-24 18:28         ` Junio C Hamano [this message]
2023-07-25 15:22           ` Christian Couder
2023-07-25 17:25             ` Junio C Hamano
2023-07-25 23:08               ` Junio C Hamano
2023-08-08  8:45                 ` Christian Couder
2023-08-09 20:38                   ` Taylor Blau
2023-08-09 22:50                   ` Junio C Hamano
2023-08-09 23:38                     ` Junio C Hamano
2023-08-10  0:10                       ` Jeff King
2023-07-05 18:12     ` Junio C Hamano
2023-07-24  9:02       ` Christian Couder
2023-07-05  6:08   ` [PATCH v2 6/8] gc: add `gc.repackFilter` config option Christian Couder
2023-07-05  6:08   ` [PATCH v2 7/8] repack: implement `--filter-to` for storing filtered out objects Christian Couder
2023-07-05 18:26     ` Junio C Hamano
2023-07-24  9:00       ` Christian Couder
2023-07-24 18:18         ` Junio C Hamano
2023-07-25 13:41           ` Robert Coup
2023-07-25 16:50             ` Junio C Hamano
2023-07-25 15:45           ` Christian Couder
2023-07-05  6:08   ` [PATCH v2 8/8] gc: add `gc.repackFilterTo` config option Christian Couder
2023-07-24  8:59   ` [PATCH v3 0/8] Repack objects into separate packfiles based on a filter Christian Couder
2023-07-24  8:59     ` [PATCH v3 1/8] pack-objects: allow `--filter` without `--stdout` Christian Couder
2023-07-25 22:38       ` Taylor Blau
2023-07-25 23:51         ` Junio C Hamano
2023-07-24  8:59     ` [PATCH v3 2/8] t/helper: add 'find-pack' test-tool Christian Couder
2023-07-25 22:44       ` Taylor Blau
2023-08-08  8:28         ` Christian Couder
2023-07-24  8:59     ` [PATCH v3 3/8] repack: refactor finishing pack-objects command Christian Couder
2023-07-25 22:45       ` Taylor Blau
2023-07-24  8:59     ` [PATCH v3 4/8] repack: refactor finding pack prefix Christian Couder
2023-07-25 22:47       ` Taylor Blau
2023-08-08  8:29         ` Christian Couder
2023-07-24  8:59     ` [PATCH v3 5/8] repack: add `--filter=<filter-spec>` option Christian Couder
2023-07-25 23:04       ` Taylor Blau
2023-08-08  8:34         ` Christian Couder
2023-08-09 21:12           ` Taylor Blau
2023-07-24  8:59     ` [PATCH v3 6/8] gc: add `gc.repackFilter` config option Christian Couder
2023-07-25 23:07       ` Taylor Blau
2023-08-08  8:38         ` Christian Couder
2023-08-09 21:15           ` Taylor Blau
2023-07-24  8:59     ` [PATCH v3 7/8] repack: implement `--filter-to` for storing filtered out objects Christian Couder
2023-07-24  8:59     ` [PATCH v3 8/8] gc: add `gc.repackFilterTo` config option Christian Couder
2023-07-25 23:10     ` [PATCH v3 0/8] Repack objects into separate packfiles based on a filter Taylor Blau
2023-08-08  8:26     ` [PATCH v4 " Christian Couder
2023-08-08  8:26       ` [PATCH v4 1/8] pack-objects: allow `--filter` without `--stdout` Christian Couder
2023-08-08  8:26       ` [PATCH v4 2/8] t/helper: add 'find-pack' test-tool Christian Couder
2023-08-09 21:18         ` Taylor Blau
2023-08-08  8:26       ` [PATCH v4 3/8] repack: refactor finishing pack-objects command Christian Couder
2023-08-08  8:26       ` [PATCH v4 4/8] repack: refactor finding pack prefix Christian Couder
2023-08-09 21:20         ` Taylor Blau
2023-08-08  8:26       ` [PATCH v4 5/8] repack: add `--filter=<filter-spec>` option Christian Couder
2023-08-09 21:40         ` Taylor Blau
2023-08-08  8:26       ` [PATCH v4 6/8] gc: add `gc.repackFilter` config option Christian Couder
2023-08-08  8:26       ` [PATCH v4 7/8] repack: implement `--filter-to` for storing filtered out objects Christian Couder
2023-08-08  8:26       ` [PATCH v4 8/8] gc: add `gc.repackFilterTo` config option Christian Couder
2023-08-09 21:45       ` [PATCH v4 0/8] Repack objects into separate packfiles based on a filter Taylor Blau
2023-08-09 21:57         ` Junio C Hamano
2023-08-12  0:12         ` Christian Couder
2023-08-12  0:00       ` [PATCH v5 " Christian Couder
2023-08-12  0:00         ` [PATCH v5 1/8] pack-objects: allow `--filter` without `--stdout` Christian Couder
2023-08-12  0:00         ` [PATCH v5 2/8] t/helper: add 'find-pack' test-tool Christian Couder
2023-08-12  0:00         ` [PATCH v5 3/8] repack: refactor finishing pack-objects command Christian Couder
2023-08-12  0:00         ` [PATCH v5 4/8] repack: refactor finding pack prefix Christian Couder
2023-08-12  0:00         ` [PATCH v5 5/8] repack: add `--filter=<filter-spec>` option Christian Couder
2023-08-12  0:00         ` [PATCH v5 6/8] gc: add `gc.repackFilter` config option Christian Couder
2023-08-12  0:00         ` [PATCH v5 7/8] repack: implement `--filter-to` for storing filtered out objects Christian Couder
2023-08-12  0:00         ` [PATCH v5 8/8] gc: add `gc.repackFilterTo` config option Christian Couder
2023-08-15  0:51         ` [PATCH v5 0/8] Repack objects into separate packfiles based on a filter Junio C Hamano
2023-08-15 21:43           ` Taylor Blau
2023-08-15 22:32             ` Junio C Hamano
2023-08-15 23:09               ` Taylor Blau
2023-08-15 23:18                 ` Junio C Hamano
2023-08-16  0:38                   ` Taylor Blau
2023-08-16 17:16                     ` Junio C Hamano
2023-09-11 15:20                 ` Christian Couder
2023-09-11 15:06         ` [PATCH v6 0/9] " Christian Couder
2023-09-11 15:06           ` [PATCH v6 1/9] pack-objects: allow `--filter` without `--stdout` Christian Couder
2023-09-11 15:06           ` [PATCH v6 2/9] t/helper: add 'find-pack' test-tool Christian Couder
2023-09-11 15:06           ` [PATCH v6 3/9] repack: refactor finishing pack-objects command Christian Couder
2023-09-11 15:06           ` [PATCH v6 4/9] repack: refactor finding pack prefix Christian Couder
2023-09-11 15:06           ` [PATCH v6 5/9] pack-bitmap-write: rebuild using new bitmap when remapping Christian Couder
2023-09-11 15:06           ` [PATCH v6 6/9] repack: add `--filter=<filter-spec>` option Christian Couder
2023-09-11 15:06           ` [PATCH v6 7/9] gc: add `gc.repackFilter` config option Christian Couder
2023-09-11 15:06           ` [PATCH v6 8/9] repack: implement `--filter-to` for storing filtered out objects Christian Couder
2023-09-11 15:06           ` [PATCH v6 9/9] gc: add `gc.repackFilterTo` config option Christian Couder
2023-09-25 15:25           ` [PATCH v7 0/9] Repack objects into separate packfiles based on a filter Christian Couder
2023-09-25 15:25             ` [PATCH v7 1/9] pack-objects: allow `--filter` without `--stdout` Christian Couder
2023-09-25 15:25             ` [PATCH v7 2/9] t/helper: add 'find-pack' test-tool Christian Couder
2023-09-25 15:25             ` [PATCH v7 3/9] repack: refactor finishing pack-objects command Christian Couder
2023-09-25 15:25             ` [PATCH v7 4/9] repack: refactor finding pack prefix Christian Couder
2023-09-25 15:25             ` [PATCH v7 5/9] pack-bitmap-write: rebuild using new bitmap when remapping Christian Couder
2023-09-25 15:25             ` [PATCH v7 6/9] repack: add `--filter=<filter-spec>` option Christian Couder
2023-09-25 15:25             ` [PATCH v7 7/9] gc: add `gc.repackFilter` config option Christian Couder
2023-09-25 15:25             ` [PATCH v7 8/9] repack: implement `--filter-to` for storing filtered out objects Christian Couder
2023-09-25 15:25             ` [PATCH v7 9/9] gc: add `gc.repackFilterTo` config option Christian Couder
2023-09-25 19:14             ` [PATCH v7 0/9] Repack objects into separate packfiles based on a filter Junio C Hamano
2023-09-25 22:41               ` Taylor Blau
2023-10-02 16:54             ` [PATCH v8 " Christian Couder
2023-10-02 16:54               ` [PATCH v8 1/9] pack-objects: allow `--filter` without `--stdout` Christian Couder
2023-10-02 16:54               ` [PATCH v8 2/9] t/helper: add 'find-pack' test-tool Christian Couder
2023-10-02 16:54               ` [PATCH v8 3/9] repack: refactor finishing pack-objects command Christian Couder
2023-10-02 16:54               ` [PATCH v8 4/9] repack: refactor finding pack prefix Christian Couder
2023-10-02 16:55               ` [PATCH v8 5/9] pack-bitmap-write: rebuild using new bitmap when remapping Christian Couder
2023-10-02 16:55               ` [PATCH v8 6/9] repack: add `--filter=<filter-spec>` option Christian Couder
2023-10-02 16:55               ` [PATCH v8 7/9] gc: add `gc.repackFilter` config option Christian Couder
2023-10-02 16:55               ` [PATCH v8 8/9] repack: implement `--filter-to` for storing filtered out objects Christian Couder
2023-10-02 16:55               ` [PATCH v8 9/9] gc: add `gc.repackFilterTo` config option Christian Couder
2023-10-02 20:14               ` [PATCH v8 0/9] Repack objects into separate packfiles based on a filter Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqila9p3j8.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johncai86@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=ps@pks.im \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).