git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Son Luong Ngoc <sluongng@gmail.com>
Cc: peff@peff.net, dstolee@microsoft.com, git@vger.kernel.org,
	me@ttaylorr.com
Subject: Re: [PATCH 3/3] commit-graph: respect 'core.useBloomFilters'
Date: Mon, 13 Jul 2020 15:22:50 -0400	[thread overview]
Message-ID: <20200713192250.GB77607@syl.lan> (raw)
In-Reply-To: <CAL3xRKdZyE+9r-bPTDo_Fiz=nT_Y7uve+rvBqQZxjL-DYMGYpw@mail.gmail.com>

On Wed, Jul 01, 2020 at 11:58:53AM +0200, Son Luong Ngoc wrote:
> Hi folks,
>
> On Tue, 30 Jun 2020 15:33:40 -0400, Jeff King wrote:
>
> > > > It might even be worth considering whether "changed paths" needs more
> > > > context (or would if we add new features in the future). On a "git
> > > > commit-graph write" command-line it is perfectly clear, but would
> > > > core.commitGraphChangedPaths be worth it? It's definitely more specific,
> > > > but it's also way more ugly. ;)
> > >
> > > Here's a third option what about 'graph.readChangedPaths'. I think that
> > > Stolee and I discussed a new top-level 'graph' section, since we now
> > > have a few commit-graph-related configuration variables in 'core'.
> >
> > Yes, I like that even better. Probably "graph" is sufficiently specific
> > within Git's context, though I guess it _could_ bring to mind "git log
> > --graph". So many overloaded terms. :)
>
> I would suggest using 'commitgraph.readChangedPaths' as I was planning on
> implementing the same config in [1] but never got around to it.
>
> From an end-user perspective, not server admin, 'graph' is very much
> correlated to 'git log --graph'.

Do users really correlate the top-level 'graph.*' configuration with
options *just* related to 'git log --graph'? I find this unlikely, but I
would welcome the opinions of others, too.

> Using 'commitgraph' instead of core could also help us enabling more config
> down the line that equate to the current options in 'git commit-graph write'.
>
> I.e. something like 'commitgraph.writeSplit' might be desirable to tune the
> behavior of 'gc.writeCommitGraph' to use '--split=replace' strategy.
>
> ---
>
> @Taylor: Thanks a lot for implementing this.
>
> On Tue, 30 Jun 2020 13:17:36 -0400, Taylor Blau wrote:
>
> > We're planning on using these patches as part of a two-phase roll-out of
> > changed-path Bloom filters, where the first phase conditions whether or
> > not repositories *write* Bloom filters, and the second phase (controlled
> > via the new 'core.useBloomFilters') controls whether repositories *read*
> > their Bloom filters.
>
> Could you elaborate a bit more on the 'two-phase roll-out' mentioned here?

Sure. What I am referring to is the ability to control independently
which repositories write Bloom filters during commit-graph generation
(via some background jobs, the details of which are unimportant), and
which repositories read Bloom filters when, for eg. running 'git log' or
similar.

This allows us to quickly recover from any sort of bug in, say, 'git
log's use of Bloom filters without having to drop the (otherwise
correct) Bloom filters from disk. In other words, it allows us to
pretend that they are not there.

> I was looking for a way to verify whether a commit-graph chain has been
> written with Bloom filter (and force it to rewrite if not) but there seems
> to be no straightforward way?

No, 'git commit-graph verify' does not deal with Bloom filters for now.
It may be worthwhile to add that functionality, though.

> Do we need to implement a flag in 'git commit-graph verify' to check
> for Bloom filter existence?

Checking for existence would be one thing. More helpful would be
regenerating those Bloom filters and checking that we get the same
result. Allowing the caller to specify which one would be helpful, too.

Thanks.

> [1]: https://github.com/gitgitgadget/git/pull/633
>
> Regards,
> Son Luong.

Thanks,
Taylor

  reply	other threads:[~2020-07-13 19:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-01  9:58 Son Luong Ngoc
2020-07-13 19:22 ` Taylor Blau [this message]
  -- strict thread matches above, loose matches on Subject: below --
2020-06-30 17:17 [PATCH 0/3] commit-graph: introduce 'core.useBloomFilters' Taylor Blau
2020-06-30 17:17 ` [PATCH 3/3] commit-graph: respect 'core.useBloomFilters' Taylor Blau
2020-06-30 19:18   ` Jeff King
2020-06-30 19:27     ` Taylor Blau
2020-06-30 19:33       ` Jeff King

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=20200713192250.GB77607@syl.lan \
    --to=me@ttaylorr.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sluongng@gmail.com \
    --subject='Re: [PATCH 3/3] commit-graph: respect '\''core.useBloomFilters'\''' \
    /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

Code repositories for project(s) associated with this 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).