{"thread":{"id":"53783","subject":"Re: [PATCH 3/3] commit-graph: respect 'core.useBloomFilters'","startedAt":"2020-07-01T09:59:07Z","lastAt":"2020-07-13T19:22:54Z","messageCount":2,"participants":["Son Luong Ngoc","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"400881","messageId":"CAL3xRKdZyE+9r-bPTDo_Fiz=nT_Y7uve+rvBqQZxjL-DYMGYpw@mail.gmail.com","threadId":"53783","inReplyTo":null,"subject":"Re: [PATCH 3/3] commit-graph: respect 'core.useBloomFilters'","fromName":"Son Luong Ngoc","fromEmail":"sluongng@gmail.com","sentAt":"2020-07-01T09:58:53Z","receivedAt":"2020-07-01T09:59:07Z","isPatch":true,"sender":{"key":"sluongng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26684313?v=4"},"body":"Hi folks,\n\nOn Tue, 30 Jun 2020 15:33:40 -0400, Jeff King wrote:\n\n> > > It might even be worth considering whether \"changed paths\" needs more\n> > > context (or would if we add new features in the future). On a \"git\n> > > commit-graph write\" command-line it is perfectly clear, but would\n> > > core.commitGraphChangedPaths be worth it? It's definitely more specific,\n> > > but it's also way more ugly. ;)\n> >\n> > Here's a third option what about 'graph.readChangedPaths'. I think that\n> > Stolee and I discussed a new top-level 'graph' section, since we now\n> > have a few commit-graph-related configuration variables in 'core'.\n>\n> Yes, I like that even better. Probably \"graph\" is sufficiently specific\n> within Git's context, though I guess it _could_ bring to mind \"git log\n> --graph\". So many overloaded terms. :)\n\nI would suggest using 'commitgraph.readChangedPaths' as I was planning on\nimplementing the same config in [1] but never got around to it.\n\nFrom an end-user perspective, not server admin, 'graph' is very much\ncorrelated to 'git log --graph'.\n\nUsing 'commitgraph' instead of core could also help us enabling more config\ndown the line that equate to the current options in 'git commit-graph write'.\n\nI.e. something like 'commitgraph.writeSplit' might be desirable to tune the\nbehavior of 'gc.writeCommitGraph' to use '--split=replace' strategy.\n\n---\n\n@Taylor: Thanks a lot for implementing this.\n\nOn Tue, 30 Jun 2020 13:17:36 -0400, Taylor Blau wrote:\n\n> We're planning on using these patches as part of a two-phase roll-out of\n> changed-path Bloom filters, where the first phase conditions whether or\n> not repositories *write* Bloom filters, and the second phase (controlled\n> via the new 'core.useBloomFilters') controls whether repositories *read*\n> their Bloom filters.\n\nCould you elaborate a bit more on the 'two-phase roll-out' mentioned here?\n\nI was looking for a way to verify whether a commit-graph chain has been\nwritten with Bloom filter (and force it to rewrite if not) but there seems\nto be no straightforward way?\n\nDo we need to implement a flag in 'git commit-graph verify' to check\nfor Bloom filter existence?\n\n[1]: https://github.com/gitgitgadget/git/pull/633\n\nRegards,\nSon Luong.\n"},{"id":"401499","messageId":"20200713192250.GB77607@syl.lan","threadId":"53783","inReplyTo":"CAL3xRKdZyE+9r-bPTDo_Fiz=nT_Y7uve+rvBqQZxjL-DYMGYpw@mail.gmail.com","subject":"Re: [PATCH 3/3] commit-graph: respect 'core.useBloomFilters'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-07-13T19:22:50Z","receivedAt":"2020-07-13T19:22:54Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jul 01, 2020 at 11:58:53AM +0200, Son Luong Ngoc wrote:\n> Hi folks,\n>\n> On Tue, 30 Jun 2020 15:33:40 -0400, Jeff King wrote:\n>\n> > > > It might even be worth considering whether \"changed paths\" needs more\n> > > > context (or would if we add new features in the future). On a \"git\n> > > > commit-graph write\" command-line it is perfectly clear, but would\n> > > > core.commitGraphChangedPaths be worth it? It's definitely more specific,\n> > > > but it's also way more ugly. ;)\n> > >\n> > > Here's a third option what about 'graph.readChangedPaths'. I think that\n> > > Stolee and I discussed a new top-level 'graph' section, since we now\n> > > have a few commit-graph-related configuration variables in 'core'.\n> >\n> > Yes, I like that even better. Probably \"graph\" is sufficiently specific\n> > within Git's context, though I guess it _could_ bring to mind \"git log\n> > --graph\". So many overloaded terms. :)\n>\n> I would suggest using 'commitgraph.readChangedPaths' as I was planning on\n> implementing the same config in [1] but never got around to it.\n>\n> From an end-user perspective, not server admin, 'graph' is very much\n> correlated to 'git log --graph'.\n\nDo users really correlate the top-level 'graph.*' configuration with\noptions *just* related to 'git log --graph'? I find this unlikely, but I\nwould welcome the opinions of others, too.\n\n> Using 'commitgraph' instead of core could also help us enabling more config\n> down the line that equate to the current options in 'git commit-graph write'.\n>\n> I.e. something like 'commitgraph.writeSplit' might be desirable to tune the\n> behavior of 'gc.writeCommitGraph' to use '--split=replace' strategy.\n>\n> ---\n>\n> @Taylor: Thanks a lot for implementing this.\n>\n> On Tue, 30 Jun 2020 13:17:36 -0400, Taylor Blau wrote:\n>\n> > We're planning on using these patches as part of a two-phase roll-out of\n> > changed-path Bloom filters, where the first phase conditions whether or\n> > not repositories *write* Bloom filters, and the second phase (controlled\n> > via the new 'core.useBloomFilters') controls whether repositories *read*\n> > their Bloom filters.\n>\n> Could you elaborate a bit more on the 'two-phase roll-out' mentioned here?\n\nSure. What I am referring to is the ability to control independently\nwhich repositories write Bloom filters during commit-graph generation\n(via some background jobs, the details of which are unimportant), and\nwhich repositories read Bloom filters when, for eg. running 'git log' or\nsimilar.\n\nThis allows us to quickly recover from any sort of bug in, say, 'git\nlog's use of Bloom filters without having to drop the (otherwise\ncorrect) Bloom filters from disk. In other words, it allows us to\npretend that they are not there.\n\n> I was looking for a way to verify whether a commit-graph chain has been\n> written with Bloom filter (and force it to rewrite if not) but there seems\n> to be no straightforward way?\n\nNo, 'git commit-graph verify' does not deal with Bloom filters for now.\nIt may be worthwhile to add that functionality, though.\n\n> Do we need to implement a flag in 'git commit-graph verify' to check\n> for Bloom filter existence?\n\nChecking for existence would be one thing. More helpful would be\nregenerating those Bloom filters and checking that we get the same\nresult. Allowing the caller to specify which one would be helpful, too.\n\nThanks.\n\n> [1]: https://github.com/gitgitgadget/git/pull/633\n>\n> Regards,\n> Son Luong.\n\nThanks,\nTaylor\n"}]}