git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 00/30] [RFC] extensions.refFormat and packed-refs v2 file format

From
Elijah Newren <newren@gmail.com>
Date
Nov 15, 2022, 02:47 UTC
Message-ID
<CABPp-BFNvUQx7exLgqDvzhgn1s=xSFKbJWdr8qfxLTXEFDQQig@mail.gmail.com>
In-Reply-To
<0e156172-0670-2832-78cb-c7dfe2599192@github.com>
On Sun, Nov 13, 2022 at 4:07 PM Derrick Stolee <derrickstolee@github.com> wrote:
Show 36 quoted lines
>
> 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:
> >>
> >> Introduction
> >> ============
> >>
> >> I became interested in our packed-ref format based on the asymmetry between
> >> ref updates and ref deletions: if we delete a packed ref, then the
> >> packed-refs file needs to be rewritten. Compared to writing a loose ref,
> >> this is an O(N) cost instead of O(1).
> >>
> >> In this way, I set out with some goals:
> >>
> >>  * (Primary) Make packed ref deletions be nearly as fast as loose ref
> >>    updates.
> >
> > Performance is always nice.  :-)
> >
> >>  * (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.
> >
> > Is this secondary goal the actual goal you have, or just the
> > implementation by which you get the real underlying goal?
>
> To me, the primary goal takes precedence. It turns out that the best
> way to solve for that goal happens to also make it possible to store
> all refs in a packed form, because we can update the packed form
> much faster than our current setup. There are alternatives that I
> considered (and prototyped) that were more specific to the deletions
> case, but they were not actually as fast as the stacked method. Those
> alternatives also would never help reach the secondary goal, but I
> probably would have considered them anyway if they were faster, if
> only for their simplicity.

That's orthogonal to my question, though. For your primary goal, you stated it in a form where it was obvious what benefit it would provide to end users. Your secondary goal, as stated, didn't list any benefit to end users that I could see (update: reading the rest of your response it appears I just didn't understand it), so I was trying to guess at why your secondary goal might be a goal, i.e. what the real secondary goal was.

Show 15 quoted lines
> > To me, it appears that such a capability would solve both (a) D/F
> > conflict problems (i.e. the ability to simultaneously have a
> > refs/heads/feature and refs/heads/feature/shiny ref), and (b) case
> > sensitivity issues in refnames (i.e. inability of some users to work
> > with both a refs/heads/feature and a refs/heads/FeAtUrE, due to
> > constraints of their filesystem and the loose storage mechanism).  Are
> > either of those the goal you are trying to achieve (I think both would
> > be really nice, more so than the performance goal you have), or is
> > there another?
>
> For a Git host provider, these D/F conflict and case-sensitivity
> situations probably would need to stay as restrictions on the
> server side for quite some time because we don't want users on
> older Git clients to be unable to fetch a repository just because
> we updated our ref storage to allow for such possibilities.

Okay, but even if not used on the server side, this capability could still be used on the client side and provide a big benefit to end users.

But I think there's a minor issue with what you stated; as far as I can tell, there is no case-sensitivity restriction on the server side for GitHub currently, and users do currently have problems cloning and using repositories with branches that differ in case only. See e.g. https://github.com/newren/git-filter-repo/issues/48 and the multiple duplicates which reference that issue. We've also had issues at $DAYJOB, though for GHE we added some hooks to deny creating branches that differ only in case from another branch to avoid the problem.

Also, D/F restrictions on the server do not stop users from having D/F problems when fetching. If users forget to use `--prune`, then when a refs/heads/foo has already been fetched is deleted and replaced by a refs/heads/foo/bar, then the user gets errors. This issue actually caused a bit of a fire-drill for us just recently.

So both kinds of problems already exist, for users with any git client version (although the former only for users with unfortunate file systems). And both problems cause pain. Both issues are caused by loose refs, so limiting git storage to packed refs would fix both issues.

Show 8 quoted lines
> 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.

Previous: Derrick StoleeNext: Derrick Stolee
Message 40 of 56 in “[RFC] extensions.refFormat and packed-refs v2 file format”
  1. 00/30 [RFC] extensions.refFormat and packed-refs v2 file formatDerrick Stolee via GitGitGadget, Nov 7, 2022
  2. 01/30 hashfile: allow skipping the hash functionDerrick Stolee via GitGitGadget, Nov 7, 2022
  3. 02/30 read-cache: add index.computeHash config optionDerrick Stolee via GitGitGadget, Nov 7, 2022
  4. Elijah NewrenNov 11, 2022
  5. Derrick StoleeNov 14, 2022
  6. Ævar Arnfjörð BjarmasonNov 17, 2022
  7. 03/30 extensions: add refFormat extensionDerrick Stolee via GitGitGadget, Nov 7, 2022
  8. Elijah NewrenNov 11, 2022
  9. Derrick StoleeNov 16, 2022
  10. 06/30 refs: allow loose files without packed-refsDerrick Stolee via GitGitGadget, Nov 7, 2022
  11. 07/30 chunk-format: number of chunks is optionalDerrick Stolee via GitGitGadget, Nov 7, 2022
  12. 04/30 config: fix multi-level bulleted listDerrick Stolee via GitGitGadget, Nov 7, 2022
  13. 05/30 repository: wire ref extensions to ref backendsDerrick Stolee via GitGitGadget, Nov 7, 2022
  14. 08/30 chunk-format: document trailing table of contentsDerrick Stolee via GitGitGadget, Nov 7, 2022
  15. 09/30 chunk-format: store chunk offset during writeDerrick Stolee via GitGitGadget, Nov 7, 2022
  16. 11/30 chunk-format: parse trailing table of contentsDerrick Stolee via GitGitGadget, Nov 7, 2022
  17. 10/30 chunk-format: allow trailing table of contentsDerrick Stolee via GitGitGadget, Nov 7, 2022
  18. 13/30 packed-backend: extract add_write_error()Derrick Stolee via GitGitGadget, Nov 7, 2022
  19. 12/30 refs: extract packfile format to new fileDerrick Stolee via GitGitGadget, Nov 7, 2022
  20. 14/30 packed-backend: extract iterator/updates mergeDerrick Stolee via GitGitGadget, Nov 7, 2022
  21. 16/30 config: add config values for packed-refs v2Derrick Stolee via GitGitGadget, Nov 7, 2022
  22. 15/30 packed-backend: create abstraction for writing refsDerrick Stolee via GitGitGadget, Nov 7, 2022
  23. 17/30 packed-backend: create shell of v2 writesDerrick Stolee via GitGitGadget, Nov 7, 2022
  24. 18/30 packed-refs: write file format version 2Derrick Stolee via GitGitGadget, Nov 7, 2022
  25. 19/30 packed-refs: read file format v2Derrick Stolee via GitGitGadget, Nov 7, 2022
  26. 20/30 packed-refs: read optional prefix chunksDerrick Stolee via GitGitGadget, Nov 7, 2022
  27. 21/30 packed-refs: write prefix chunksDerrick Stolee via GitGitGadget, Nov 7, 2022
  28. 22/30 packed-backend: create GIT_TEST_PACKED_REFS_VERSIONDerrick Stolee via GitGitGadget, Nov 7, 2022
  29. 24/30 t5312: allow packed-refs v2 formatDerrick Stolee via GitGitGadget, Nov 7, 2022
  30. 23/30 t1409: test with packed-refs v2Derrick Stolee via GitGitGadget, Nov 7, 2022
  31. 26/30 t3210: require packed-refs v1 for some testsDerrick Stolee via GitGitGadget, Nov 7, 2022
  32. 25/30 t5502: add PACKED_REFS_V1 prerequisiteDerrick Stolee via GitGitGadget, Nov 7, 2022
  33. 27/30 t*: skip packed-refs v2 over http testsDerrick Stolee via GitGitGadget, Nov 7, 2022
  34. 28/30 ci: run GIT_TEST_PACKED_REFS_VERSION=2 in some buildsDerrick Stolee via GitGitGadget, Nov 7, 2022
  35. 29/30 p1401: create performance test for ref operationsDerrick Stolee via GitGitGadget, Nov 7, 2022
  36. 30/30 refs: skip hashing when writing packed-refs v2Derrick Stolee via GitGitGadget, Nov 7, 2022
  37. Derrick StoleeNov 9, 2022
  38. Elijah NewrenNov 11, 2022
  39. Derrick StoleeNov 14, 2022
  40. Elijah NewrenNov 15, 2022
  41. Derrick StoleeNov 16, 2022
  42. Elijah NewrenNov 17, 2022
  43. Junio C HamanoNov 18, 2022
  44. Elijah NewrenNov 19, 2022
  45. Taylor BlauNov 19, 2022
  46. Derrick StoleeNov 30, 2022
  47. Han-Wen NienhuysNov 28, 2022
  48. Derrick StoleeNov 30, 2022
  49. Phillip WoodNov 30, 2022
  50. Taylor BlauNov 30, 2022
  51. Han-Wen NienhuysNov 30, 2022
  52. Sean AllredNov 30, 2022
  53. Derrick StoleeDec 1, 2022
  54. Han-Wen NienhuysDec 2, 2022
  55. Ævar Arnfjörð BjarmasonDec 2, 2022
  56. Junio C HamanoNov 30, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.