[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1092660 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c crop.c wc_db.c wc_db.h

From: Greg Stein <gstein_at_gmail.com>
Date: Fri, 15 Apr 2011 15:59:32 -0400

On Fri, Apr 15, 2011 at 15:46, Bert Huijben <bert_at_qqmail.nl> wrote:
>> -----Original Message-----
>> From: Greg Stein [mailto:gstein_at_gmail.com]
>> Sent: vrijdag 15 april 2011 20:51
>> To: dev_at_subversion.apache.org
>> Subject: Re: svn commit: r1092660 - in
>> /subversion/trunk/subversion/libsvn_wc: adm_ops.c crop.c wc_db.c
>> wc_db.h
>> On Fri, Apr 15, 2011 at 07:03,  <rhuijben_at_apache.org> wrote:
>> >...
>> > Instead of installing a wq operation to remove records from the database
>> > in the commit processing, just perform the operation directly.
>> >
>> > Working queue operations are for synchronizing the working copy with the
>> > database. In this case the working copy doesn't change at all.
>> They are also used to ensure that N separate database operations are
>> all completed. You don't want the deletes to happen without the rest
>> of the commit processing, too.
>> Given that... does this change still make sense? I'm not really sure
>> of the node removal within the larger context of the commit.
> Yes. It removes only the shadowed operations, which is safe
>  Once the commit for this node has been processed the shadowed operations
> below this layer are invalid by itself, as they rely on their op_roots,
> which are being removed by the processing. (And this happens in the same
> transaction as the removal of that op_root). So it actually avoids db
> instability.
> Before this patch series we did:
> Queue one operation
> Queue one operation
> Run operations
> (db is in an unstable situation here)
> Queue one operation
> Run one operation,
> (etc)

Oh, I totally agree with the delay of the queue-run. That was an
artifact of multi-db operation (and the old loggy files).

> (and some of these operations were not restartable yet)
> But why queue db-only changes when we can just perform them directly with
> the same atomic guarantees?
> (The SQLite transaction guarantees that the db_op_commit_node is completely
> evaluated or not at all; same guarantee as queuing a single wq operation to
> perform that operation)

Well.. it looked like the op_remove_node was called from
process_committed_leaf. That is outside any guarantees.

On the other hand, db_global_commit is called from within the
workqueue. Thus, "all" commits should be getting processed during the

> The code shifting now even allows moving all the processing (and the queuing
> of the wq operations) in a single sqlite transaction which would improve
> stability and performance even further.

I agree with the sentiment, and I do agree that the commit processing
is not our ideal of "db changes and queue items in one txn". It is
kind of backwards now as a result of the old loggy style, and before I
had really settled on how wc_db and wq should interact (the transacted
insertion of items, for example).

But I still don't think this change is right. You immediately remove
nodes, or you queue up a commit operation (in order to batch the
entire commit). What am I missing here?

Received on 2011-04-15 22:00:03 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.