Julian Foad wrote on Mon, 12 Oct 2009 at 12:24 +0100:
> > Daniel Shahaf wrote on Sat, 10 Oct 2009 at 14:31 +0200:
> > > Julian Foad wrote on Sat, 10 Oct 2009 at 01:55 +0100:
> > > > 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)
> > > > +{
...
> > > > + if (ffd->rep_sharing_allowed)
> > > > + {
...
> > > > + }
> > > > +
> > > > + SVN_ERR(svn_fs_fs__with_write_lock(fs, commit_obliteration_body, &cb, pool));
> > > > +
> > > > + if (ffd->rep_sharing_allowed)
> > > > + {
...
> > > > + }
> > > > +
> > >
> > > [ 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.
>
Thanks for explaining. However, I'm not much more familiar with FSFS
than you are --- I read a bit, and fixed a few bugs, but that only
scratches the surface.
> > > 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.
Representations are stored in the rev files. The DB only stores the
coordinates (offsets into rev files) of representations, keyed by the
sha1 of the file generated by the representation.
Commits that want to use a representation write out its coordinates in
full in the revision file they create. (In particular, they *do not*
refer to the DB; this is why the latter can be removed at any point.)
For example, in a Greek tree (r1) with 'svn ps foo bar iota' (r2) and
'/bin/cp iota iota2; svn add iota2' (r3):
% sha1sum < wc1/trunk/iota
2c0aa9014a0cd07f01795a333d82485ef6d083e2 -
% sqlite3 r/db/rep-cache.db "select * from rep_cache where hash = '2c0aa9014a0cd07f01795a333d82485ef6d083e2';"
2c0aa9014a0cd07f01795a333d82485ef6d083e2|1|547|37|25
### [1]
% grep -ar 2c0aa9014a0cd07f01795a333d82485ef6d083e2 r/db/revs
r/db/revs/0.pack/pack:text: 1 547 37 25 2d18c5e57e84c5b8a5e9a6e13fa394dc 2c0aa9014a0cd07f01795a333d82485ef6d083e2 0-0/_16
r/db/revs/0.pack/pack:text: 1 547 37 25 2d18c5e57e84c5b8a5e9a6e13fa394dc 2c0aa9014a0cd07f01795a333d82485ef6d083e2 0-0/_16
r/db/revs/0.pack/pack:text: 1 547 37 25 2d18c5e57e84c5b8a5e9a6e13fa394dc 2c0aa9014a0cd07f01795a333d82485ef6d083e2 2-2/_3
### [2]
# that's the representation
% xxd -s 347 -l 50 r/db/revs/0/1
0000223: 4445 4c54 410a 5356 4e01 0000 1902 1a01 DELTA.SVN.......
0000233: 9919 5468 6973 2069 7320 7468 6520 6669 ..This is the fi
0000243: 6c65 2027 696f 7461 272e 0a45 4e44 5245 le 'iota'..ENDRE
0000253: 500a P.
Daniel
(The docs I got this info from are the 'Revision file format' section of
the FSFS 'structure' file.)
[1] There is a pack file because I build with PACK_AFTER_EVERY_COMMIT and
with SVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR=4. A normal build would
see matches in revs/0/{1,2,3}.
[2] 50 == 37 + strlen("DELTA\n") + strlen("ENDREP\n")
> > 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=2406817
Received on 2009-10-13 23:14:35 CEST