git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Stefan Beller <sbeller@google.com>,
	Junio C Hamano <gitster@pobox.com>,
	Ronnie Sahlberg <sahlberg@google.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH v3 00/14] ref-transactions-reflog
Date: Thu, 20 Nov 2014 10:17:01 -0800	[thread overview]
Message-ID: <20141120181701.GB15945@google.com> (raw)
In-Reply-To: <546DC8E9.6090504@alum.mit.edu>

Michael Haggerty wrote:
> On 11/20/2014 12:22 AM, Stefan Beller wrote:

>> 3. Delete the reflog when the corresponding reference is deleted [1].
>>
>> also as one transaction?
>
> It would be a side-effect of committing a transaction that contains a
> reference deletion. The deletion of the reflog would be done at the same
> time that the rest of the transaction is committed, and again the
> calling code wouldn't have to explicitly worry about the reflogs.

There is "git reflog delete <ref>", for when you have logallrefupdates
disabled and had explicitly enabled reflogs for a particular ref and
now want to turn it off.

[...]
> So this design has the caller serializing all reflog entries into
> separate ref_update structs (which implies that they are held in RAM!)
> only for ref_transaction_commit() to scan through all ref_updates
> looking for reflog updates that go together so that they can be
> processed as a whole. In other words, the caller picks the reflog apart
> and then ref_transaction_commit() glues it back together. It's all very
> contrived.

I think there is a simpler and more efficient way to implement this.

transaction_update_reflog() can append to a .lock file.
transaction_commit() then would rename it into place.

There is some fuss about naming the .lock file to avoid D/F conflicts,
which is a topic for a separate message.

> I suggest that the caller only be responsible for deciding which reflog
> entries to keep (by supplying a callback function),

That could be handy.  The basic operations described before would still
be needed, though:

	create a new reflog with one entry, for new refs

	append an entry to a reflog, for ref updates (and the associated
		symref reflog update)

	copy (or rename --- that's a more minor detail) a reflog, for
		renaming refs

	delete a reflog, for "git reflog delete"

And the "filter reflog" operation you are describing is implementable
using those four operations, with no performance hit when dealing with
reflogs stored in the files backend.

Providing these operations doesn't prevent adding "filter reflog using
callback" later if it turns out to be the right operation for other
backends.  It could turn out that some other primitive operations that
are easy as an SQL operation is more useful, like "delete reflog
entry" (without iterating over the others) or "expire entries older
than <date>".  The nice thing is that adding those wouldn't break any
code using the initial four operations described above.  So this seems
like a good starting point.

[...]
> I would love to work on this but unfortunately have way too much on my
> plate right now.

Of course code is always an easy way to change my mind, when the time
comes. ;-)

Thanks,
Jonathan

  reply	other threads:[~2014-11-20 18:17 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-18  1:35 [PATCH v3 00/14] ref-transactions-reflog Stefan Beller
2014-11-18  1:35 ` [PATCH v3 01/14] refs.c: make ref_transaction_create a wrapper for ref_transaction_update Stefan Beller
2014-11-18  1:35 ` [PATCH v3 02/14] refs.c: make ref_transaction_delete " Stefan Beller
2014-11-18  1:35 ` [PATCH v3 03/14] refs.c: rename the transaction functions Stefan Beller
2014-11-18  1:35 ` [PATCH v3 04/14] refs.c: add a function to append a reflog entry to a fd Stefan Beller
2014-11-18  1:35 ` [PATCH v3 05/14] refs.c: add a new update_type field to ref_update Stefan Beller
2014-11-18  1:35 ` [PATCH v3 06/14] refs.c: add a transaction function to append a reflog entry Stefan Beller
2014-11-18  1:35 ` [PATCH v3 07/14] refs.c: add a flag to allow reflog updates to truncate the log Stefan Beller
2014-11-18  1:35 ` [PATCH v3 08/14] refs.c: only write reflog update if msg is non-NULL Stefan Beller
2014-11-18  1:35 ` [PATCH v3 09/14] refs.c: allow multiple reflog updates during a single transaction Stefan Beller
2014-11-18  1:35 ` [PATCH v3 10/14] reflog.c: use a reflog transaction when writing during expire Stefan Beller
2014-11-18  1:35 ` [PATCH v3 11/14] refs.c: rename log_ref_setup to create_reflog Stefan Beller
2014-11-18  1:35 ` [PATCH v3 12/14] refs.c: Remove unlock_ref/close_ref/commit_ref from the refs api Stefan Beller
2014-11-18  1:35 ` [PATCH v3 13/14] refs.c: remove lock_any_ref_for_update Stefan Beller
2014-11-18  1:35 ` [PATCH v3 14/14] refs.c: allow deleting refs with a broken sha1 Stefan Beller
2014-11-18 11:26 ` [PATCH v3 00/14] ref-transactions-reflog Michael Haggerty
2014-11-18 18:36   ` Ronnie Sahlberg
2014-11-18 19:46     ` Michael Haggerty
2014-11-18 20:30       ` Junio C Hamano
2014-11-18 21:16         ` Michael Haggerty
2014-11-18 21:28           ` Junio C Hamano
2014-11-19 23:22             ` Stefan Beller
2014-11-20  3:24               ` Jonathan Nieder
2014-11-20 17:34                 ` Junio C Hamano
2014-11-20 10:56               ` Michael Haggerty
2014-11-20 18:17                 ` Jonathan Nieder [this message]
2014-11-27  5:34                   ` [PATCH 0/4] Using transactions for the reflog Stefan Beller
2014-11-27  5:34                     ` [PATCH 1/4] refs.c: rename the transaction functions Stefan Beller
2014-11-27  5:34                     ` [PATCH 2/4] refs.c: add a new update_type field to ref_update Stefan Beller
2014-11-27  5:34                     ` [PATCH 3/4] refs.c: add a transaction function to append a reflog entry Stefan Beller
2014-11-27  5:34                     ` [PATCH 4/4] reflog.c: use a reflog transaction when writing during expire Stefan Beller
  -- strict thread matches above, loose matches on Subject: below --
2014-06-18 17:08 [PATCH v3 00/14] ref-transactions-reflog Ronnie Sahlberg

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141120181701.GB15945@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    --cc=sahlberg@google.com \
    --cc=sbeller@google.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.
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 NNTP newsgroup(s).