[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: David Glasser <glasser_at_davidglasser.net>
Date: Mon, 12 Oct 2009 09:21:50 -0700

For what it's worth, the current schema for the rep-sharing DB is
completely incompatible with "remove data from server" obliterate
(though it's certainly compatible with "mask data from clients"
obliterate).

--dave

On Mon, Oct 12, 2009 at 4:24 AM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> Daniel Shahaf wrote:
>> [ couldn't sleep ]
>
> Heh. That's sometimes when the clearest thoughts come.
>
>> 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. ]
>
> Thanks for saying this: I don't yet have this level of awareness. I'm
> learning the whole repository and FSFS side as I go along. I'm very
> grateful for your eyes on this.
>
>> > 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).
>
> When you say "cache", is it a cache in the sense that functionally it
> doesn't matter whether it contains any or all of these things? From what
> you ssay below, it sounds like it is a "rep store". Please let me
> understand that clearly before I go on.
>
>> 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.
>
>
> Thanks for the other comments, which I believe I have addressed in what
> I committed in r39949.
>
> - Julian
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406584
>

-- 
glasser_at_davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2406720
Received on 2009-10-12 18:22:20 CEST

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