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

Re: obliterate in trunk (was: svn commit: r39745 - trunk/subversion/svn)

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sun, 11 Oct 2009 03:56:22 +0200 (Jerusalem Standard Time)

[ couldn't sleep ]

Daniel Shahaf wrote on Sat, 10 Oct 2009 at 14:31 +0200:
> Julian Foad wrote on Sat, 10 Oct 2009 at 01:55 +0100:
> > Add an initial skeleton implementation for "svn obliterate", with the UI
> > and public API changes hidden behind #ifdef SVN_WITH_EXPERIMENTAL_OBLITERATE.
> >
> > ...
> >
> > Index: subversion/libsvn_fs_fs/fs_fs.c
> > ===================================================================
> > --- subversion/libsvn_fs_fs/fs_fs.c (revision 39914)
> > +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> > @@ -5885,6 +6025,45 @@ svn_fs_fs__commit(svn_revnum_t *new_rev_
> > }
> >
> > svn_error_t *
> > +svn_fs_fs__commit_obliteration(svn_revnum_t rev,
> > + svn_fs_t *fs,
> > + svn_fs_txn_t *txn,
> > + apr_pool_t *pool)
> > +{
> > + struct commit_baton cb;
> > + fs_fs_data_t *ffd = fs->fsap_data;
> > +
> > + /* Analogous to svn_fs_fs__commit(). */
> > + cb.new_rev_p = &rev;
> > + cb.fs = fs;
> > + cb.txn = txn;
> > +
> > + if (ffd->rep_sharing_allowed)
> > + {
> > + cb.reps_to_cache = apr_array_make(pool, 5, sizeof(representation_t *));
> > + cb.reps_pool = pool;
> > + }
> > + else
> > + {
> > + cb.reps_to_cache = NULL;
> > + cb.reps_pool = NULL;
> > + }
> > +
> > + SVN_ERR(svn_fs_fs__with_write_lock(fs, commit_obliteration_body, &cb, pool));
> > +
> > + if (ffd->rep_sharing_allowed)
> > + {
> > + /* ### TODO: ignore errors opening the DB (issue #3506) * */
> > + SVN_ERR(svn_fs_fs__open_rep_cache(fs, pool));
> > + SVN_ERR(svn_sqlite__with_transaction(ffd->rep_cache_db,
> > + commit_sqlite_txn_callback,
> > + &cb, pool));
> > + }
> > +
>
> [ I think you know this already, but I'll say it anyway. ]
>
> Half of the lines of this function deal with storing new reps in the
> rep-cache DB. Some day it should also remove the old DB rows (and
> somehow cause references in other revisions into the revision-being-
> obliterated to still work).
>

Actually, after r39897, it's possible that DB rows referencing the
revision-being-obliterated[1] will be added at an arbitrary time after
that revision has been committed: it's (theoretically, depending on
the order SQLite hands out write locks) possible that the sequence

    # make a few large (multi-file) commits
    # commit rBO
    # obliterate rBO
    # wait 3 seconds

will result in rep-cache rows referring the obliterated version of rBO.

Not sure how to solve that. We want to ensure that the obliterate doesn't
get the SQLite write lock before "its" commit gets the same lock. [2]

Possible damage? If the DB rows relating to the original rBO are added
after its obliteration its complete, then reps created in the future may
rely on these rows and avoid writing themselves out explicitly --- even
though the reps may no longer be in the rBO rev file.

Daniel

[1] this needs an acronym, it's too long to type every time.
[2] theoretically I'm used to assuming different processes have arbitrary
    relative speeds, which makes the situation even worse (it forbids
    assuming that the commit process has even requested the SQLite write
    lock).
[3] http://www.sqlite.org/lockingv3.html

> > + return SVN_NO_ERROR; +}

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406286
Received on 2009-10-11 03:56:29 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.