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
queue-run.
> 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?
Cheers,
-g
Received on 2011-04-15 22:00:03 CEST