[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1099657 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c wc_db.c wc_db.h

From: Greg Stein <gstein_at_gmail.com>
Date: Thu, 5 May 2011 02:20:09 -0400

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.

Received on 2011-05-05 08:20:52 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.