{"thread":{"id":"53426","subject":"How to move forward with Reftable","startedAt":"2020-05-07T18:51:18Z","lastAt":"2020-05-12T23:37:50Z","messageCount":4,"participants":["Han-Wen Nienhuys","Jonathan Nieder","Junio C Hamano","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"397332","messageId":"CAFQ2z_Ne-1TWMRS88ADhzQmx4OfzNEkVUJ1anjf57mQD4Gdywg@mail.gmail.com","threadId":"53426","inReplyTo":null,"subject":"How to move forward with Reftable","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2020-05-07T18:51:02Z","receivedAt":"2020-05-07T18:51:18Z","isPatch":false,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"Hi all,\n\nI have noticed that not everyone is happy with the reftable series as\nit currently stands. I wanted to give some background on why it looks\nthe way it does today, I want to solicit feedback on where to go from\nhere, and share my worries about the negative feedback I’ve gotten so\nfar.\n\nI manage the Gerrit team at Google. Gerrit uses refs extensively: we\nhave many repositories with millions of refs. To address the problems\nthis caused for Google, Shawn designed the reftable storage format,\nand we have been running it in production since 2017. Unfortunately,\nthis doesn’t solve the problem for ‘normal’ deployments of Gerrit, and\nthis limits design choices that my team can make. To fix this, I have\nimplemented support for reftable on local file systems in JGit, and\nwant to make it a feature that is supported by core git too.\n\nI found the implementation somewhat gnarly, which makes it an\ninteresting project. I thought it would be good for the ecosystem if\nrelated projects such as libgit2 could use the same implementation, so\nI wrote it as a library that is completely standalone (except for a\ndependency on zlib). It was originally written in Go, translated to as\nidiomatic C as I could manage. It is basically object-oriented with a\nsmall amount of polymorphism, so it should translate relatively easily\nto languages like Python.\n\nAs it currently stands, the library implements the specification\ncompletely, including the parts that are currently probably not useful\nfor Git, like support for the object-ID/ref mapping. The API between\nreftable and git is extremely narrow: it’s about 20 functions, and 5\nstructure definitions. It’s also well tested (see coverage [2]. As of\ntoday, the tests are leak-free according to valgrind). This is also\nwhy I don’t believe Johannes’ argument that, because he found one bug,\nit must be full of bugs.\n\nThis approach has been successful, in the sense that the libgit2\nproject has experimented with integrating it, see\nhttps://github.com/libgit2/libgit2/pull/5462. They seem happy at the\nprospect of integrating existing code rather than reimplementing it.\n\nJohannes had suggested that this should be developed and maintained in\ngit-core first, and the result could then be reused by libgit2\nproject. According to the libgit2 folks, this what that would look\nlike:\n\n“””\n    - It needs to be easy to split out from git-core. If it is\n      self-contained in a single directory, then I'd be sufficiently\n      happy already.\n\n    - It should continue providing a clean interface to external\n      callers. This also means that its interface should stay stable so\n      we don't have to adapt on every update. git-core historically\n      never had such promises, but it kind of worked out for the xdiff\n      code.\n\n    - My most important fear would be that the reftable interface\n      becomes heavily dependent on git-core's own data types. We had\n      this discussion at the Contributor's Summit already, but if it\n      starts adopting things like git-core's own string buffer then it\n      would become a lot harder for us to use it.\n\n    - Probably obvious, but contained in the above is that it stays\n      compilable on its own. So even if you split out its directory and\n      wire up some build instructions, it should not have any\n      dependencies on git-core.\n”””\n\n(for the discussion at the summit:\nhttps://lore.kernel.org/git/1B71B54C-E000-4CEB-8AC6-3DB86E96E31A@jramsay.com.au/)\n\nI can make that work, but it would be good to know if this is\nsomething the project is OK with in principle, or whether the code\nneeds to be completely Git-ified. If the latter happens, that would\neffectively fork the code, which I think is a missed opportunity.\n\nI have received a lot of negative comments, and I can’t gauge well\nwhat to make of them, but they kindle several worries:\n\n* The git community decides they don’t need the object reverse index,\nand do not write ‘o’ section in reftables, because that culls 1000\nlines of code. This generally works, but will cause hard to debug\nperformance regressions if command-line git is used on a gerrit\nserver.\n\n* The git community looks at this, and decides the standard is too\ncomplex, and goes off to create a reftable v3.\n\n* The git community asks me to do a ton of superficial work (eg. slice\n-> strbuf), and then decides the overall design needs to be different,\nand should be completely rewritten.\n\nJonathan Nieder said I shouldn’t worry about standards compliance,\nbecause the Git project has already ratified the reftable standard,\nand wouldn’t want to break JGit compatibility, but it would be good to\nhave the community leaders reaffirm this stance.\n\nFor my last worry, it would be good if someone would make an\nassessment of the overall design to see if it is acceptable.\n\nOnce we have a path forward we can think of a way of integrating the\ncode. I think it may take a while to shake out bugs. Not bugs in the\nreftable library itself, but Git is not very strict in enforcing\nproper use of the ref backend abstraction (eg. pseudorefs [1]). Many\nintegration tests also don’t go through proper channels for accessing\nrefs.\n\ncheers,\n\n\n[1] https://lore.kernel.org/git/CAFQ2z_NZgkPE+3oazfb_m0_7TWxHjje1yYCc0bMZG05_KUKEow@mail.gmail.com/\n\n[2] https://hanwen.home.xs4all.nl/public/software/reftable-coverage/\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"397562","messageId":"20200511183602.GA141481@google.com","threadId":"53426","inReplyTo":"CAFQ2z_Ne-1TWMRS88ADhzQmx4OfzNEkVUJ1anjf57mQD4Gdywg@mail.gmail.com","subject":"Re: How to move forward with Reftable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2020-05-11T18:36:02Z","receivedAt":"2020-05-11T18:36:07Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(+cc: a few people I'm particularly interested in feedback from)\n\nHan-Wen Nienhuys wrote[3]:\n\n> I have noticed that not everyone is happy with the reftable series as\n> it currently stands. I wanted to give some background on why it looks\n> the way it does today, I want to solicit feedback on where to go from\n> here, and share my worries about the negative feedback I've gotten so\n> far.\n[...]\n> As it currently stands, the library implements the specification\n> completely, including the parts that are currently probably not useful\n> for Git, like support for the object-ID/ref mapping. The API between\n> reftable and git is extremely narrow: it's about 20 functions, and 5\n> structure definitions. It's also well tested (see coverage [2]. As of\n> today, the tests are leak-free according to valgrind). This is also\n> why I don't believe Johannes' argument that, because he found one bug,\n> it must be full of bugs.\n>\n> This approach has been successful, in the sense that the libgit2\n> project has experimented with integrating it, see\n> https://github.com/libgit2/libgit2/pull/5462. They seem happy at the\n> prospect of integrating existing code rather than reimplementing it.\n\nSeparate from the integration aspect is that this is not yet\nbattle-tested code.  One benefit of sharing code is to be able to share\nthe benefit of users testing it.\n\nSince the ref system is fairly modular and this is about a non-default\nbackend, it's likely okay to integrate it initially as \"experimental\"\nand then update docs as we gain confidence.\n\n> Johannes had suggested that this should be developed and maintained in\n> git-core first, and the result could then be reused by libgit2\n> project. According to the libgit2 folks, this what that would look\n> like:\n>\n> \"\"\"\n>     - It needs to be easy to split out from git-core. If it is\n>       self-contained in a single directory, then I'd be sufficiently\n>       happy already.\n>\n>     - It should continue providing a clean interface to external\n>       callers. This also means that its interface should stay stable so\n>       we don't have to adapt on every update. git-core historically\n>       never had such promises, but it kind of worked out for the xdiff\n>       code.\n>\n>     - My most important fear would be that the reftable interface\n>       becomes heavily dependent on git-core's own data types. We had\n>       this discussion at the Contributor's Summit already, but if it\n>       starts adopting things like git-core's own string buffer then it\n>       would become a lot harder for us to use it.\n>\n>     - Probably obvious, but contained in the above is that it stays\n>       compilable on its own. So even if you split out its directory and\n>       wire up some build instructions, it should not have any\n>       dependencies on git-core.\n> \"\"\"\n>\n> (for the discussion at the summit:\n> https://lore.kernel.org/git/1B71B54C-E000-4CEB-8AC6-3DB86E96E31A@jramsay.com.au/)\n>\n> I can make that work, but it would be good to know if this is\n> something the project is OK with in principle, or whether the code\n> needs to be completely Git-ified. If the latter happens, that would\n> effectively fork the code, which I think is a missed opportunity.\n\nThere's been some discussion about use of strbuf versus the module's\nown growing-buffer \"struct slice\".  Is that the only instance of this\nkind of infrastructure duplication or are there others?\n\n> I have received a lot of negative comments, and I can't gauge well\n> what to make of them, but they kindle several worries:\n>\n> * The git community decides they don't need the object reverse index,\n>   and do not write 'o' section in reftables, because that culls 1000\n>   lines of code. This generally works, but will cause hard to debug\n>   performance regressions if command-line git is used on a gerrit\n>   server.\n>\n> * The git community looks at this, and decides the standard is too\n>   complex, and goes off to create a reftable v3.\n>\n> * The git community asks me to do a ton of superficial work (eg. slice\n>   -> strbuf), and then decides the overall design needs to be different,\n>   and should be completely rewritten.\n>\n> Jonathan Nieder said I shouldn't worry about standards compliance,\n> because the Git project has already ratified the reftable standard,\n> and wouldn't want to break JGit compatibility, but it would be good to\n> have the community leaders reaffirm this stance.\n\nRecording from today's IRC standup for easy reference later:\n\n  <gitster> jrnieder: that might have been solved already by me declaring\n  that whether they like it or not, reftable must be in git-core to match\n  what JGit has.\n\n> For my last worry, it would be good if someone would make an\n> assessment of the overall design to see if it is acceptable.\n\nThanks.  I think Peff did a little in this area; help from others\nwould be welcome as well.\n\n> Once we have a path forward we can think of a way of integrating the\n> code. I think it may take a while to shake out bugs. Not bugs in the\n> reftable library itself, but Git is not very strict in enforcing\n> proper use of the ref backend abstraction (eg. pseudorefs [1]). Many\n> integration tests also don't go through proper channels for accessing\n> refs.\n\nYes, I'm *very* excited that the series includes a knob for running\nthe testsuite using the reftable ref backend, providing a way for\nanyone interested to pitch in to help with the issues they reveal\n(both in Git and in its testsuite).\n\nSummary: thanks for writing this.  I personally think your proposal is\na reasonable one, but I'm looking forward to hearing from others as\nwell.  I'm also looking forward to discussing \"struct slice\" and the\noverall architecture in the context of the patches[4] and making some\nmore progress.\n\nSincerely,\nJonathan\n\n> cheers,\n>\n> [1] https://lore.kernel.org/git/CAFQ2z_NZgkPE+3oazfb_m0_7TWxHjje1yYCc0bMZG05_KUKEow@mail.gmail.com/\n>\n> [2] https://hanwen.home.xs4all.nl/public/software/reftable-coverage/\n\n[3] https://lore.kernel.org/git/CAFQ2z_Ne-1TWMRS88ADhzQmx4OfzNEkVUJ1anjf57mQD4Gdywg@mail.gmail.com/\n\n[4] https://lore.kernel.org/git/pull.539.v12.git.1588845585.gitgitgadget@gmail.com/\n"},{"id":"397563","messageId":"xmqqpnbaqpaw.fsf@gitster.c.googlers.com","threadId":"53426","inReplyTo":"20200511183602.GA141481@google.com","subject":"Re: How to move forward with Reftable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-11T18:50:15Z","receivedAt":"2020-05-11T18:50:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> (+cc: a few people I'm particularly interested in feedback from)\n> ...\n> There's been some discussion about use of strbuf versus the module's\n> own growing-buffer \"struct slice\".  Is that the only instance of this\n> kind of infrastructure duplication or are there others?\n\nYeah, that is a very important question.  If there are own APIs that\nare similar to what we have but impedance mismatch can not be easily\nabstracted away, that would be a huge problem.\n\n"},{"id":"397639","messageId":"20200512233741.GB6605@camp.crustytoothpaste.net","threadId":"53426","inReplyTo":"20200511183602.GA141481@google.com","subject":"Re: How to move forward with Reftable","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-05-12T23:37:41Z","receivedAt":"2020-05-12T23:37:50Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-05-11 at 18:36:02, Jonathan Nieder wrote:\n> Separate from the integration aspect is that this is not yet\n> battle-tested code.  One benefit of sharing code is to be able to share\n> the benefit of users testing it.\n> \n> Since the ref system is fairly modular and this is about a non-default\n> backend, it's likely okay to integrate it initially as \"experimental\"\n> and then update docs as we gain confidence.\n\nIf we're going to integrate it, I would like to pass the testsuite when\nwe do.  We can certainly do a series of preparatory patches (e.g., to\nthe testsuite) to get it ready, but once people can turn it on or use it\n(via config, I assume), it should be fully functional and tested.\n\nHaving said that, I agree we should mark it as experimental first, at\nleast for a while.  I'm interested to see how it works both from a\nfunctionality perspective as well as a performance perspective.  For\nexample, is reftable a win with a relatively large number of refs (say,\ntens of thousands)?  Operational experience will tell us that and help\nguide us to figure out if and when it should be the default.\n\n> > Johannes had suggested that this should be developed and maintained in\n> > git-core first, and the result could then be reused by libgit2\n> > project. According to the libgit2 folks, this what that would look\n> > like:\n> >\n> > \"\"\"\n> >     - It needs to be easy to split out from git-core. If it is\n> >       self-contained in a single directory, then I'd be sufficiently\n> >       happy already.\n> >\n> >     - It should continue providing a clean interface to external\n> >       callers. This also means that its interface should stay stable so\n> >       we don't have to adapt on every update. git-core historically\n> >       never had such promises, but it kind of worked out for the xdiff\n> >       code.\n> >\n> >     - My most important fear would be that the reftable interface\n> >       becomes heavily dependent on git-core's own data types. We had\n> >       this discussion at the Contributor's Summit already, but if it\n> >       starts adopting things like git-core's own string buffer then it\n> >       would become a lot harder for us to use it.\n> >\n> >     - Probably obvious, but contained in the above is that it stays\n> >       compilable on its own. So even if you split out its directory and\n> >       wire up some build instructions, it should not have any\n> >       dependencies on git-core.\n> > \"\"\"\n> >\n> > (for the discussion at the summit:\n> > https://lore.kernel.org/git/1B71B54C-E000-4CEB-8AC6-3DB86E96E31A@jramsay.com.au/)\n> >\n> > I can make that work, but it would be good to know if this is\n> > something the project is OK with in principle, or whether the code\n> > needs to be completely Git-ified. If the latter happens, that would\n> > effectively fork the code, which I think is a missed opportunity.\n> \n> There's been some discussion about use of strbuf versus the module's\n> own growing-buffer \"struct slice\".  Is that the only instance of this\n> kind of infrastructure duplication or are there others?\n\nThere's duplication of the hash algorithm stuff.  I don't know what else\nbecause I haven't taken an in-depth look at the code other than for\nSHA-256 compatibility.  I think Dscho has more insight here.\n\nIn general, my view here is that if this is going to be a part of core\nGit, then it should live in core Git and use our standard tooling.  If\nthis is going to be a logically independent shared library (like zlib)\nor an optional feature that one can compile with or not, then it can\nlive outside of the tree as a separate project (and shared library) that\nwe link against.\n\nWe've already seen a bunch of compatibility pain from sha1dc, which has\na much smaller, more well-defined interface.  I'd like to not repeat\nthat behavior with the reftable code, especially since Git runs on a\nwide variety of systems and has significant compatibility needs.\n\nI also don't love the fact that we have an update script that overwrites\nall of our changes with the upstream code when there are some of us who\nhave no intention of contributing to upstream (e.g., for CLA reasons).\nBarring some way of addressing those concerns, I think we're going to\nneed to assume that updates require some sort of manual rebase work like\nwith other code that we import.\n\n> Yes, I'm *very* excited that the series includes a knob for running\n> the testsuite using the reftable ref backend, providing a way for\n> anyone interested to pitch in to help with the issues they reveal\n> (both in Git and in its testsuite).\n\nI think this is a good feature to have and definitely the right way\nforward.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"}]}