On Fri, Apr 15, 2011 at 15:46, Bert Huijben <b...@qqmail.nl> wrote: >> -----Original Message----- >> From: Greg Stein [mailto:gst...@gmail.com] >> Sent: vrijdag 15 april 2011 20:51 >> To: dev@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, <rhuij...@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
