One alternative solution is having svn_fs_fs__verify() stat() the DB
file for existence before attempting to operate on it.
I don't think it's the most elegant thing in the world, but it avoids
spuriously creating the DB file without complicating the common case
logic.
I'm also open to alternative suggestions if anyone has them.
Julian Foad wrote on Tue, Jul 19, 2011 at 19:12:33 +0100:
> On Thu, 2011-07-14, Daniel Shahaf wrote:
> > I've looked again at the code. Right now it opens the db, with
> > svn_sqlite__mode_rwcreate, within an svn_atomic_init_once() block.
> >
> > Adding the special case "For <this> caller use svn_sqlite__mode_readwrite"
> > means we can't use svn_atomic_init_once() any more. (since it's no
> > longer "once", maybe the first call failed due to being non-rwcreate)
> > And have to add some error handling logic around the open() call in that
> > function.
> >
> > How about going for the less elegant but more readable solution,
> >
> > [[[
> > Index: subversion/libsvn_fs_fs/rep-cache.c
> > ===================================================================
> > @@ -182,6 +187,11 @@ svn_fs_fs__walk_rep_reference(svn_fs_t *fs,
> > }
> > +
> > + if (iterations == 0)
> > + SVN_ERR(svn_io_remove_file2(path_rep_cache_db(fs->path, iterpool),
> > + TRUE /* ignore_enoent */,
> > + iterpool));
> >
> > SVN_ERR(svn_sqlite__reset(stmt));
> >
> > ]]]
> >
> > ?
>
> I've just looked at the code to see what you're suggesting here. You're
> suggesting to make svn_fs_fs__walk_rep_reference() delete the rep cache
> DB file if it's empty. Relying on the fact that svn_fs_fs__verify() is
> currently the only caller of that function. That seems too ugly.
>
> - Julian
>
>
> > > > We could make the code pass readwrite (rather than rwcreate) for that
> > > > one caller, and then mask the error when the DB doesn't exist.
> > >
> > > +1.
> > >
> > > > (We do
> > > > want to verify the DB if it's exists but fsfs.conf says "Don't use it",
> > > > since fsfs.conf may get changed.)
> > >
> > > +1.
> > >
> > > > I'm not sure that non-creating an
> > > > unused db justifies this additional code.
> > >
> > > I'm with Greg - it would be good for the users.
> > >
> > > - Julian
>
>
Received on 2011-07-19 23:00:54 CEST