You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2018/09/28 16:08:06 UTC

Re: svn commit: r1838813 - in /subversion/trunk/subversion: libsvn_fs_fs/recovery.c tests/cmdline/svnadmin_tests.py

julianfoad@apache.org wrote on Fri, 24 Aug 2018 10:48 +0000:
> Author: julianfoad
> Date: Fri Aug 24 10:48:37 2018
> New Revision: 1838813
> 
> URL: http://svn.apache.org/viewvc?rev=1838813&view=rev
> Log:
> Let 'svnadmin recover' prune the rep-cache even if it is disabled.
> 
> Part of issue #4077: "FSFS recover should prune unborn revisions from
> rep-cache.db".
> 
> Also add tests for both cases (enabled, disabled).
> 
> Pruning the rep-cache even if disabled was included as r1213716 in the
> original fix for issue #4077, first released in Subversion 1.7.3, but was
> reverted in r1367674 (issue #4214, recovery should not create rep-cache.db)
> and so omitted from Subversion 1.8 and later series of releases.
> 
> * subversion/libsvn_fs_fs/recovery.c
>   (recover_body): Prune the rep-cache no matter whether it's in use.
> 
> * subversion/tests/cmdline/svnadmin_tests.py
>   (read_rep_cache,
>    check_recover_prunes_rep_cache): New functions.
>   (recover_prunes_rep_cache_when_enabled,
>    recover_prunes_rep_cache_when_disabled): New tests.
>   (test_list): Run them.
> 
> +++ subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
> @@ -53,6 +53,23 @@ Wimp = svntest.testcase.Wimp_deco
>  SkipDumpLoadCrossCheck = svntest.testcase.SkipDumpLoadCrossCheck_deco
>  Item = svntest.wc.StateItem
>  
> +def read_rep_cache(repo_dir):
> +  """Return the rep-cache contents as a dict {hash: (rev, index, ...)}.
> +  """
> +  db_path = os.path.join(repo_dir, 'db', 'rep-cache.db')

Should this function assert that db/format <= 8?  When we introduce
format 9 it might not be compatible with this function's expectations.

> +  db1 = svntest.sqlite3.connect(db_path)
> +  schema1 = db1.execute("pragma user_version").fetchone()[0]
> +  # Can't test newer rep-cache schemas with an old built-in SQLite.
> +  if schema1 >= 2 and svntest.sqlite3.sqlite_version_info < (3, 8, 2):
> +    raise svntest.Failure("Can't read rep-cache schema %d using old "
> +                          "Python-SQLite version %s < (3,8,2)" %
> +                           (schema1,
> +                            svntest.sqlite3.sqlite_version_info))
> +
> +  content = { row[0]: row[1:] for row in
> +              db1.execute("select * from rep_cache") }
> +  return content

Re: svn commit: r1838813 - in /subversion/trunk/subversion: libsvn_fs_fs/recovery.c tests/cmdline/svnadmin_tests.py

Posted by Julian Foad <ju...@foad.me.uk>.
Daniel Shahaf wrote:
> julianfoad@apache.org wrote on Fri, 24 Aug 2018 10:48 +0000:
> > Author: julianfoad
> > Date: Fri Aug 24 10:48:37 2018
> > New Revision: 1838813
> > 
> > URL: http://svn.apache.org/viewvc?rev=1838813&view=rev
> > Log:
> > Let 'svnadmin recover' prune the rep-cache even if it is disabled.
> > 
> > Part of issue #4077: "FSFS recover should prune unborn revisions from
> > rep-cache.db".
> > 
> > Also add tests for both cases (enabled, disabled).
> > 
> > Pruning the rep-cache even if disabled was included as r1213716 in the
> > original fix for issue #4077, first released in Subversion 1.7.3, but was
> > reverted in r1367674 (issue #4214, recovery should not create rep-cache.db)
> > and so omitted from Subversion 1.8 and later series of releases.
> > 
> > * subversion/libsvn_fs_fs/recovery.c
> >   (recover_body): Prune the rep-cache no matter whether it's in use.
> > 
> > * subversion/tests/cmdline/svnadmin_tests.py
> >   (read_rep_cache,
> >    check_recover_prunes_rep_cache): New functions.
> >   (recover_prunes_rep_cache_when_enabled,
> >    recover_prunes_rep_cache_when_disabled): New tests.
> >   (test_list): Run them.
> > 
> > +++ subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
> > @@ -53,6 +53,23 @@ Wimp = svntest.testcase.Wimp_deco
> >  SkipDumpLoadCrossCheck = svntest.testcase.SkipDumpLoadCrossCheck_deco
> >  Item = svntest.wc.StateItem
> >  
> > +def read_rep_cache(repo_dir):
> > +  """Return the rep-cache contents as a dict {hash: (rev, index, ...)}.
> > +  """
> > +  db_path = os.path.join(repo_dir, 'db', 'rep-cache.db')
> 
> Should this function assert that db/format <= 8?  When we introduce
> format 9 it might not be compatible with this function's expectations.

I suppose that would be a way to enforce that we revisit this logic then. On the other hand we don't generally do that for anything else (FSFS formats, WC formats, etc.).

No strong opinion.

> > +  db1 = svntest.sqlite3.connect(db_path)
> > +  schema1 = db1.execute("pragma user_version").fetchone()[0]
> > +  # Can't test newer rep-cache schemas with an old built-in SQLite.
> > +  if schema1 >= 2 and svntest.sqlite3.sqlite_version_info < (3, 8, 2):
> > +    raise svntest.Failure("Can't read rep-cache schema %d using old "
> > +                          "Python-SQLite version %s < (3,8,2)" %
> > +                           (schema1,
> > +                            svntest.sqlite3.sqlite_version_info))
> > +
> > +  content = { row[0]: row[1:] for row in
> > +              db1.execute("select * from rep_cache") }
> > +  return content

-- 
- Julian