git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee <derrickstolee@github.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, jrnieder@gmail.com
Subject: Re: [PATCH 00/30] [RFC] extensions.refFormat and packed-refs v2 file format
Date: Wed, 16 Nov 2022 20:28:00 -0800	[thread overview]
Message-ID: <CABPp-BFsvZeC34=VKN9ir+KM0tx4rt0eiGuyKzrD=OAi9sABNw@mail.gmail.com> (raw)
In-Reply-To: <01063560-8f57-4e40-5707-f8d8ecfe6cca@github.com>

On Wed, Nov 16, 2022 at 6:45 AM Derrick Stolee <derrickstolee@github.com> wrote:
>
> On 11/14/22 9:47 PM, Elijah Newren wrote:
> > On Sun, Nov 13, 2022 at 4:07 PM Derrick Stolee <derrickstolee@github.com> wrote:
> >>
> >> On 11/11/22 6:28 PM, Elijah Newren wrote:
> >>> On Mon, Nov 7, 2022 at 11:01 AM Derrick Stolee via GitGitGadget
> >>> <gitgitgadget@gmail.com> wrote:
[...]
> >>>>  * (Secondary) Allow using a packed ref format for all refs, dropping loose
> >>>>    refs and creating a clear way to snapshot all refs at a given point in
> >>>>    time.
[...]
>
> The reason is in the goal "creating a clear way to snapshot all refs
> at a given point in time". This is a server-side benefit with no
> visible benefit to users, immediately.

Yes, sorry, I just missed it.  I didn't understand it and wrongly
assumed it was continuing to talk about the implementation details
rather than the benefit details.  My bad.

Thanks for patiently correcting me.

> The D/F conflicts and case-sensitive parts that could fall from that
> are not included in my goals. Part of that is because we would need a
> new reflog format to complete that part. Let's take things one step
> at a time and handle reflogs after we have ref update performance
> handled.

Ah, right, I can see how reflog would affect both of those problems
now that you highlight it, but it hadn't occurred to me before.

> >> The biggest benefit on the server side is actually for consistency
> >> checks. Using a stacked packed-refs (especially with a tip file
> >> that describes all of the layers) allows an atomic way to take a
> >> snapshot of the refs and run a checksum operation on their values.
> >> With loose refs, concurrent updates can modify the checksum during
> >> its computation. This is a super niche reason for this, but it's
> >> nice that the performance-only focus also ends up with a design
> >> that satisfies this goal.
> >
> > Ah...so this is the reason for your secondary goal?  Re-reading it
> > looks like you did state this, I just missed it without the longer
> > explanation.
> >
> > Anyway, it might be worth calling out in your cover letter that there
> > are (at least) three benefits to this secondary goal of yours -- the
> > one you list here, plus the two I list above.
>
> I suppose I assumed that the D/F and case conflicts were a "known"
> benefit and a huge motivation of the reftable work.

Yes, and I thought you had just found a simpler solution to those
problems that might not provide all the benefits of reftable (e.g.
performance with huge numbers of refs) but did solve those particular
problems.  I've only looked at reftable from the surface from a
distance, and I was unaware previously that reflog also affected these
two problems (though it seems obvious in hindsight).  And I do
remember you calling out that you weren't changing the reflog format
in your cover letter, but I didn't understand the ramifications of
that statement at the time.

> Instead of trying
> to solve all of the ref problems at once, I wanted to focus on the
> subset that I knew could be solved with a simpler solution, leaving
> the full solution to later steps. It would help to be explicit about
> how this direction helps solve this problem while also being clear
> about how it does not solve it completely.

It certainly would have helped me.  :-)

Thanks for explaining all these details.

  reply	other threads:[~2022-11-17  4:29 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-07 18:35 [PATCH 00/30] [RFC] extensions.refFormat and packed-refs v2 file format Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 01/30] hashfile: allow skipping the hash function Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 02/30] read-cache: add index.computeHash config option Derrick Stolee via GitGitGadget
2022-11-11 23:31   ` Elijah Newren
2022-11-14 16:30     ` Derrick Stolee
2022-11-17 16:13   ` Ævar Arnfjörð Bjarmason
2022-11-07 18:35 ` [PATCH 03/30] extensions: add refFormat extension Derrick Stolee via GitGitGadget
2022-11-11 23:39   ` Elijah Newren
2022-11-16 14:37     ` Derrick Stolee
2022-11-07 18:35 ` [PATCH 04/30] config: fix multi-level bulleted list Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 05/30] repository: wire ref extensions to ref backends Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 06/30] refs: allow loose files without packed-refs Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 07/30] chunk-format: number of chunks is optional Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 08/30] chunk-format: document trailing table of contents Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 09/30] chunk-format: store chunk offset during write Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 10/30] chunk-format: allow trailing table of contents Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 11/30] chunk-format: parse " Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 12/30] refs: extract packfile format to new file Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 13/30] packed-backend: extract add_write_error() Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 14/30] packed-backend: extract iterator/updates merge Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 15/30] packed-backend: create abstraction for writing refs Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 16/30] config: add config values for packed-refs v2 Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 17/30] packed-backend: create shell of v2 writes Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 18/30] packed-refs: write file format version 2 Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 19/30] packed-refs: read file format v2 Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 20/30] packed-refs: read optional prefix chunks Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 21/30] packed-refs: write " Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 22/30] packed-backend: create GIT_TEST_PACKED_REFS_VERSION Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 23/30] t1409: test with packed-refs v2 Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 24/30] t5312: allow packed-refs v2 format Derrick Stolee via GitGitGadget
2022-11-07 18:35 ` [PATCH 25/30] t5502: add PACKED_REFS_V1 prerequisite Derrick Stolee via GitGitGadget
2022-11-07 18:36 ` [PATCH 26/30] t3210: require packed-refs v1 for some tests Derrick Stolee via GitGitGadget
2022-11-07 18:36 ` [PATCH 27/30] t*: skip packed-refs v2 over http tests Derrick Stolee via GitGitGadget
2022-11-07 18:36 ` [PATCH 28/30] ci: run GIT_TEST_PACKED_REFS_VERSION=2 in some builds Derrick Stolee via GitGitGadget
2022-11-07 18:36 ` [PATCH 29/30] p1401: create performance test for ref operations Derrick Stolee via GitGitGadget
2022-11-07 18:36 ` [PATCH 30/30] refs: skip hashing when writing packed-refs v2 Derrick Stolee via GitGitGadget
2022-11-09 15:15 ` [PATCH 00/30] [RFC] extensions.refFormat and packed-refs v2 file format Derrick Stolee
2022-11-11 23:28 ` Elijah Newren
2022-11-14  0:07   ` Derrick Stolee
2022-11-15  2:47     ` Elijah Newren
2022-11-16 14:45       ` Derrick Stolee
2022-11-17  4:28         ` Elijah Newren [this message]
2022-11-18 23:31     ` Junio C Hamano
2022-11-19  0:41       ` Elijah Newren
2022-11-19  3:00         ` Taylor Blau
2022-11-30 15:31       ` Derrick Stolee
2022-11-28 18:56 ` Han-Wen Nienhuys
2022-11-30 15:16   ` Derrick Stolee
2022-11-30 15:38     ` Phillip Wood
2022-11-30 16:37     ` Taylor Blau
2022-11-30 18:30     ` Han-Wen Nienhuys
2022-11-30 18:37       ` Sean Allred
2022-12-01 20:18       ` Derrick Stolee
2022-12-02 16:46         ` Han-Wen Nienhuys
2022-12-02 18:24           ` Ævar Arnfjörð Bjarmason
2022-11-30 22:55     ` Junio C Hamano

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='CABPp-BFsvZeC34=VKN9ir+KM0tx4rt0eiGuyKzrD=OAi9sABNw@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jrnieder@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).