Hyrum K Wright wrote on Fri, Jul 01, 2011 at 11:21:58 -0500:
> On Fri, Jul 1, 2011 at 8:05 AM, Daniel Shahaf <danielsh_at_elego.de> wrote:
> > Hyrum K Wright wrote on Thu, Jun 30, 2011 at 16:33:16 -0500:
> >> On Thu, Jun 30, 2011 at 3:27 PM, Peter Samuelson <peter_at_p12n.org> wrote:
> >> >
> >> > [Ivan Zhakov]
> >> >> It should be easy to implement editing revprops without using SQLite:
> >> >> in case someone modify revprop non-packed revprop file is created, in
> >> >> read operation non-packed revprop file should be considered as more
> >> >> up-to-date. In next svnadmin pack operation these non-packed files
> >> >> should be merged back to packed one.
> >> >
> >> > +1. This would basically mean there's only _one_ code path for writing
> >> > revprops, yes? 'svnadmin pack' gets a little more complex, but the
> >> > rest of libsvn_fs_fs gets simpler.
> >> >
> >> > Anyone have time to actually do this? Converting the packed format
> >> > from sqlite to the same format used for packed revs would be a bonus.
> >>
> >> I like this idea, but it would seem to introduce an additional stat()
> >> call* for every attempt to fetch a revprop, because you'd first have
> >> to check the "old" location, and then the packed one.
> >
> > I don't like the idea of writing revprops outside the DB and moving them
> > back in. (I think I already said why elsethread?)
>
> (I will assume "the DB" means "the SQLite-backed revprop database".)
>
> I agree with you, but I don't think that's what the proposal was
> about. My understanding would be that we'd pack revprops just like we
> pack revision (one single concat'd file per shard), and then store any
> edits in separate files. We'd then "repack" the edits into the pack
> files when an admin subsequently runs 'svnadmin pack'.
Yes, that's exactly what I don't like :-)
I already outlined one case where I think the behaviour would be
incorrect. (And even stefan2's suggestion from IRC yesterday doesn't
solve that race condition --- unless we can depend on the OS providing
an atomic unlink_if_timestamp_is_not_X() functionality)
> Aside from the complexity of having to attempt to open the non-packed
> file everywhere before failing through to the common case, I really
> like this solution. (So much so that I may go implement it...)
As I said on #3944, I plan to implement the suggestion of "Either all
revprops are in sharded files, or all revprops are in the DB" (due to
Peter). So, let's agree on /what/ behaviour we want to implement,
before either of us goes and implements anything :-)
>
> -Hyrum
Received on 2011-07-01 18:30:49 CEST