On Wed, May 4, 2011 at 8:46 PM, <gst...@apache.org> wrote: > Author: gstein > Date: Thu May 5 01:46:31 2011 > New Revision: 1099657 > > URL: http://svn.apache.org/viewvc?rev=1099657&view=rev > Log: > Combine the changelist modification notification into the operation > itself, so that (in the future) we can make guarantees about dropping the > temporary table. Add cancellation support, too. > > Add a missing clear of the iterpool in db_op_delete. > > Leave markers for future unification. > > * subversion/libsvn_wc/wc_db.h: > (svn_wc__db_op_set_chnagelist): rename a couple parameters (that > differed by a single character) for clarity. add notification and > cancellation parameters. > (svn_wc__db_changelist_list_notify): remove > > * subversion/libsvn_wc/wc_db.c: > (svn_wc__db_op_set_changelist): combine with ... > (svn_wc__db_changelist_list_notify): ... this. leave some comments. > adjust a bit of pool usage since we have an iterpool that can be used > as a better scratch_pool in the early part of the function. early-exit > if there is no NOTIFY_FUNC. fix an implicit 64-bit to 32-bit > conversion for the ACTION localvar. add cancellation. > (svn_wc__db_op_delete): clear the iterpool, and adjust some localvar > initialization to after that call. > > * subversion/libsvn_wc/adm_ops.c: > (add_from_disk, changelist_walker): shift the notification directly into > the call to db_op_set_changelist. > > Modified: > subversion/trunk/subversion/libsvn_wc/adm_ops.c > subversion/trunk/subversion/libsvn_wc/wc_db.c > subversion/trunk/subversion/libsvn_wc/wc_db.h
What does it mean to cancel during notification? Historically, notifications were interleaved with the operations, and if a user cancelled during the notification, they cancelled the remaining bits of the operation as well (leaving the wc in some possibly funky state). To the user, notification *was* the operation. You've now changed those semantics. Even though a user may cancel in the middle of a set of notifications, they haven't really interrupted the operation. As a result, they may never get a notification for those operations, even thought they were successful, and committed to the DB. (The notification items are still pending in the DB, but we've got no way to retrieve them.) We ought to think carefully about introducing this change of behavior. And part of me wonders why notifications would be long running enough to require cancellation, particularly since we are no longer interleaving them with Real Work. -Hyrum
