On Thu, May 5, 2011 at 02:00, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> On Wed, May 4, 2011 at 8:46 PM, <gstein_at_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
Received on 2011-05-05 08:20:52 CEST