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
===================================================================
--- subversion/libsvn_fs_fs/rep-cache.c (revision 1146757)
+++ subversion/libsvn_fs_fs/rep-cache.c (working copy)
@@ -43,8 +43,13 @@ REP_CACHE_DB_SQL_DECLARE_STATEMENTS(statements);
/** Helper functions. **/
+static APR_INLINE const char *
+path_rep_cache_db(const char *fs_path,
+ apr_pool_t *result_pool)
+{
+ return svn_dirent_join(fs_path, REP_CACHE_DB_NAME, result_pool);
+}
-
/* Check that REP refers to a revision that exists in FS. */
static svn_error_t *
rep_has_been_born(representation_t *rep,
@@ -91,7 +96,7 @@ open_rep_cache(void *baton,
/* Open (or create) the sqlite database. It will be automatically
closed when fs->pool is destoyed. */
- db_path = svn_dirent_join(fs->path, REP_CACHE_DB_NAME, pool);
+ db_path = path_rep_cache_db(fs->path, pool);
SVN_ERR(svn_sqlite__open(&ffd->rep_cache_db, db_path,
svn_sqlite__mode_rwcreate, statements,
0, NULL,
@@ -182,6 +187,11 @@ svn_fs_fs__walk_rep_reference(svn_fs_t *fs,
SVN_ERR(svn_sqlite__step(&have_row, stmt));
}
+
+ 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));
]]]
?
Julian Foad wrote on Thu, Jul 14, 2011 at 12:00:09 +0100:
> On Thu, 2011-07-14 at 04:10 +0300, Daniel Shahaf wrote:
> > Greg Stein wrote on Wed, Jul 13, 2011 at 20:54:56 -0400:
> > > On Wed, Jul 13, 2011 at 20:32, <danielsh_at_apache.org> wrote:
> > > > Author: danielsh
> > > > Date: Thu Jul 14 00:32:05 2011
> > > > New Revision: 1146528
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=1146528&view=rev
> > > > Log:
> > > > * subversion/libsvn_fs_fs/fs_fs.c
> > > > (write_config): Document that 'svnadmin verify' will access (and thus create)
> > > > rep-cache.db regardless of whether rep-sharing (of new reps) is enabled
> > > > in fsfs.conf.
> > >
> > > If rep-sharing is turned off, then it seems wrong to spontaneously
> > > create a .db that isn't begin used.
> >
> > It's harmless (it isn't being used).
>
> Harmless in functional terms only.
>
> > 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-14 17:58:32 CEST