git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Ronnie Sahlberg <sahlberg@google.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH v4 0/3] Make update refs more atomic
Date: Tue, 15 Apr 2014 22:32:01 +0200	[thread overview]
Message-ID: <534D9741.3010404@alum.mit.edu> (raw)
In-Reply-To: <CAL=YDWmm1pDtNuibs5CrPTDkaxT9PUvZscXFicoNsNpXVXJv1A@mail.gmail.com>

On 04/15/2014 06:33 PM, Ronnie Sahlberg wrote:
> On Mon, Apr 14, 2014 at 11:36 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> [...]
>> I wonder, however, whether your approach of changing callers from
>>
>>     lock = lock_ref_sha1_basic() (or varient of)
>>     write_ref_sha1(lock)
>>
>> to
>>
>>     lock = lock_ref_sha1_basic() (or varient of)
>>     write_ref_sha1(lock)
>>     unlock_ref(lock) | commit_ref_lock(lock)
>>
>> is not doing work that we will soon need to rework.  Would it be jumping
>> the gun to change the callers to
>>
>>     transaction = ref_transaction_begin();
>>     ref_transaction_{update,delete,etc}(transaction, ...);
>>     ref_transaction_{commit,rollback}(transaction, ...);
>>
>> instead?  Then we could bury the details of calling write_ref_sha1() and
>> commit_lock_ref() inside ref_transaction_commit() rather than having to
>> expose them in the public API.
> 
> I think you are right.
> 
> Lets put this patch series on the backburner for now and start by
> making all callers use transactions
> and remove write_ref_sha1() from the public API thar refs.c exports.
> 
> Once everything is switched over to transactions I can rework this
> patchseries for ref_transaction_commit()
> and resubmit to the mailing list.

Sounds good.  Rewriting callers to use transactions would be a great
next step.  Please especially keep track of what new features the
transactions API still needs.  More flexible error handling?  The
ability to have steps in the transaction that are "best-effort" (i.e.,
don't abort the transaction if they fail)?  Different reflog messages
for different updates within the same transaction rather than one reflog
message for all updates?  Etc.

And some callers who currently change multiple references one at a time
might be able to be rewritten to update the references in a single
transaction.

> Lets start preparing patches to change all external callers to use
> transactions instead.
> I am happy to help preparing patches for this. How do we ensure that
> we do not create duplicate work
> and work on the same functions?

I have a few loose ends to take care of on my lockfile patch series, and
there are a few things I would like to tidy up internal to the
transactions implementation, so I think if you are working on the caller
side then we won't step on each other's toes too much in the near future.

I suggest we use IRC (mhagger@freenode) or XMPP (mhagger@jabber.org) for
small-scale coordination.  I also have a GitHub repo
(http://github.com/mhagger/git) to which I often push intermediate
results; I will try to push to that more regularly (warning: I often
rebase feature branches even after they are pushed to GitHub).  I think
you are in Pacific Time whereas I am in Berlin, so we will tend to work
in serial rather than in parallel; that should help.  It would be a good
habit to shoot each short status emails at the end of each working day.

Of course we should only use one-on-one communication for early work; as
soon as something is getting ripe we should make sure our technical
discussions take place here on the mailing list.

Sound OK?
Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

  reply	other threads:[~2014-04-15 20:32 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-14 18:29 [PATCH v4 0/3] Make update refs more atomic Ronnie Sahlberg
2014-04-14 18:29 ` [PATCH v4 1/3] refs.c: split writing and commiting a ref into two separate functions Ronnie Sahlberg
2014-04-15 11:17   ` Michael Haggerty
2014-04-14 18:29 ` [PATCH v4 2/3] refs.c: split delete_ref_loose() into a separate flag-for-deletion and commit phase Ronnie Sahlberg
2014-04-15 17:19   ` Michael Haggerty
2014-04-14 18:29 ` [PATCH v4 3/3] refs.c: change ref_transaction_commit to run the commit loops once all work is finished Ronnie Sahlberg
2014-04-14 20:24 ` [PATCH v4 0/3] Make update refs more atomic Junio C Hamano
2014-04-15 16:41   ` Ronnie Sahlberg
2014-04-15  6:36 ` Michael Haggerty
2014-04-15 16:33   ` Ronnie Sahlberg
2014-04-15 20:32     ` Michael Haggerty [this message]
2014-04-16 17:11       ` Ronnie Sahlberg
2014-04-16 19:31         ` Junio C Hamano
2014-04-16 21:31           ` Ronnie Sahlberg
2014-04-16 21:42             ` Junio C Hamano
2014-04-16 21:51           ` Michael Haggerty

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=534D9741.3010404@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=sahlberg@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).