[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: Hyrum K Wright <hyrum_at_hyrumwright.org>
Date: Thu, 5 May 2011 01:00:23 -0500

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.

-Hyrum
Received on 2011-05-05 08:00:59 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.