On Thu, May 5, 2011 at 02:00, Hyrum K Wright <hy...@hyrumwright.org> wrote: > 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.
We don't know what the client is doing in those notificaitons. Could be anything. Could take any amount of time. I do recognize your point. Hitting ^C (or a "Cancel" button in a UI) could have two meanings: 1) stop! don't complete that operation! 2) shut up already. I want to do something else. I've always viewed cancellation as "stop a long-running operation". Since we normally trap ^C, there is no real way for a user to get control unless we give them a chance. Even the workqueue supports cancellation (which really means: suspend; the workqueue has to complete running some time in the future). Ideally, the notification process is fast, but somebody left a question of putting a cancel func in that processing, and it sounded reasonable. We just don't know what that callback is doing. Cheers, -g
