You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <rh...@sharpsvn.net> on 2009/10/20 20:01:17 UTC
RE: svn commit: r40135 - trunk/subversion/libsvn_wc
> -----Original Message-----
> From: Hyrum K. Wright [mailto:hyrum@hyrumwright.org]
> Sent: dinsdag 20 oktober 2009 20:17
> To: svn@subversion.tigris.org
> Subject: svn commit: r40135 - trunk/subversion/libsvn_wc
>
> Author: hwright
> Date: Tue Oct 20 11:17:13 2009
> New Revision: 40135
>
> Log:
> Remove the log_accum parameter from the loggy delete lock function, and
> instead just put the item directly into the work queue (making sure to
> flush an accumulated items before doing so).
>
> * subversion/libsvn_wc/adm_ops.c
> (process_committed_leaf): Provide a wc_db to loggy_delete_lock(), and
> remove the post-call log accum flush.
>
> * subversion/libsvn_wc/update_editor.c
> (accumulate_entry_props): Same.
>
> * subversion/libsvn_wc/log.c
> (svn_wc__loggy_delete_lock): Switch the log accumulator for a wc_db,
> and
> write the item directly to the work queue.
>
> * subversion/libsvn_wc/log.h
> (svn_wc__loggy_delete_lock): Switch the log accumulator for a wc_db,
> and
> update docs.
>
> Modified:
> trunk/subversion/libsvn_wc/adm_ops.c
> trunk/subversion/libsvn_wc/log.c
> trunk/subversion/libsvn_wc/log.h
> trunk/subversion/libsvn_wc/update_editor.c
>
> Modified: trunk/subversion/libsvn_wc/adm_ops.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/adm_ops.c?p
> athrev=40135&r1=40134&r2=40135
> =======================================================================
> =======
> --- trunk/subversion/libsvn_wc/adm_ops.c Tue Oct 20 11:09:23 2009
> (r40134)
> +++ trunk/subversion/libsvn_wc/adm_ops.c Tue Oct 20 11:17:13 2009
> (r40135)
> @@ -513,9 +513,8 @@ process_committed_leaf(svn_wc__db_t *db,
> SVN_WC__FLUSH_LOG_ACCUM(db, adm_abspath, log_accum, scratch_pool);
>
> if (remove_lock)
> - SVN_ERR(svn_wc__loggy_delete_lock(&log_accum, adm_abspath,
> + SVN_ERR(svn_wc__loggy_delete_lock(db, adm_abspath,
> path, scratch_pool,
> scratch_pool));
> - SVN_WC__FLUSH_LOG_ACCUM(db, adm_abspath, log_accum, scratch_pool);
I don't know if this is a safe transformation, but previously the log_accum
and the delete were queued at once in an all or nothing operation.
(Transactioned).
We can now fail between the flushing of the log and the queueing of the
queuing of the delete lock.
These two operations should be added to the wq in one transaction or
everything should be in one wq operation if we don't want to change the
behavior.
(I didn't review the other similar changes yet.. some other cases could be
fine.. or have a similar issue)
Bert
>
> /* ### messed up right now. we need to pass this boolean. */
> if (remove_changelist && !using_ng)
>
> Modified: trunk/subversion/libsvn_wc/log.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/log.c?pathr
> ev=40135&r1=40134&r2=40135
> =======================================================================
> =======
> --- trunk/subversion/libsvn_wc/log.c Tue Oct 20 11:09:23 2009
> (r40134)
> +++ trunk/subversion/libsvn_wc/log.c Tue Oct 20 11:17:13 2009
> (r40135)
> @@ -1531,21 +1531,23 @@ svn_wc__loggy_delete_entry(svn_stringbuf
> }
>
> svn_error_t *
> -svn_wc__loggy_delete_lock(svn_stringbuf_t **log_accum,
> +svn_wc__loggy_delete_lock(svn_wc__db_t *db,
> const char *adm_abspath,
> const char *path,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool)
> {
> const char *loggy_path1;
> + svn_stringbuf_t *buf = NULL;
>
> SVN_ERR(loggy_path(&loggy_path1, path, adm_abspath, scratch_pool));
> - svn_xml_make_open_tag(log_accum, result_pool, svn_xml_self_closing,
> + svn_xml_make_open_tag(&buf, result_pool, svn_xml_self_closing,
> SVN_WC__LOG_DELETE_LOCK,
> SVN_WC__LOG_ATTR_NAME, loggy_path1,
> NULL);
>
> - return SVN_NO_ERROR;
> + return svn_error_return(svn_wc__wq_add_loggy(db, adm_abspath, buf,
> + scratch_pool));
> }
>
> svn_error_t *
>
> Modified: trunk/subversion/libsvn_wc/log.h
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/log.h?pathr
> ev=40135&r1=40134&r2=40135
> =======================================================================
> =======
> --- trunk/subversion/libsvn_wc/log.h Tue Oct 20 11:09:23 2009
> (r40134)
> +++ trunk/subversion/libsvn_wc/log.h Tue Oct 20 11:17:13 2009
> (r40135)
> @@ -150,7 +150,7 @@ svn_wc__loggy_delete_entry(svn_stringbuf
> apr_pool_t *scratch_pool);
>
>
> -/* Extend **LOG_ACCUM with log instructions to delete lock related
> +/* Insert into DB a work queue instruction to delete lock related
> fields from the entry belonging to PATH.
> ADM_ABSPATH is the absolute path for the access baton for PATH.
>
> @@ -158,7 +158,7 @@ svn_wc__loggy_delete_entry(svn_stringbuf
> temporary allocations.
> */
> svn_error_t *
> -svn_wc__loggy_delete_lock(svn_stringbuf_t **log_accum,
> +svn_wc__loggy_delete_lock(svn_wc__db_t *db,
> const char *adm_abspath,
> const char *path,
> apr_pool_t *result_pool,
>
> Modified: trunk/subversion/libsvn_wc/update_editor.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/update_edit
> or.c?pathrev=40135&r1=40134&r2=40135
> =======================================================================
> =======
> --- trunk/subversion/libsvn_wc/update_editor.c Tue Oct 20 11:09:23
2009
> (r40134)
> +++ trunk/subversion/libsvn_wc/update_editor.c Tue Oct 20 11:17:13
2009
> (r40135)
> @@ -1177,9 +1177,8 @@ accumulate_entry_props(svn_stringbuf_t *
> if (! strcmp(prop->name, SVN_PROP_ENTRY_LOCK_TOKEN))
> {
> SVN_WC__FLUSH_LOG_ACCUM(db, adm_abspath, log_accum, pool);
> - SVN_ERR(svn_wc__loggy_delete_lock(&log_accum, adm_abspath,
> + SVN_ERR(svn_wc__loggy_delete_lock(db, adm_abspath,
> path, pool, pool));
> - SVN_WC__FLUSH_LOG_ACCUM(db, adm_abspath, log_accum, pool);
>
> if (lock_state)
> *lock_state = svn_wc_notify_lock_state_unlocked;
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
> d=2409488
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409519
Re: svn commit: r40135 - trunk/subversion/libsvn_wc
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Oct 20, 2009 at 16:01, Bert Huijben <rh...@sharpsvn.net> wrote:
>...
> I don't know if this is a safe transformation, but previously the log_accum
> and the delete were queued at once in an all or nothing operation.
> (Transactioned).
>
> We can now fail between the flushing of the log and the queueing of the
> queuing of the delete lock.
>
> These two operations should be added to the wq in one transaction or
> everything should be in one wq operation if we don't want to change the
> behavior.
While it is true that we have reduced the atomicity of
queue-insertions, this work is simply a stepping stone. We'll be
migrating to larger/higher-level-semantics for work queue operations
at some point. Further, we'll get them inserted into the queue,
transactionally, as we change other, related metadata in the database.
But that is a little ways off, and these kinds of changes will make it
easier for us to get there. We'll just live with this slightly higher
potential for mid-queueing errors leaving partial work in the queue.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409527