You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/05/04 17:14:13 UTC

svn commit: r940898 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/pristine-store-test.c

Author: julianfoad
Date: Tue May  4 15:14:13 2010
New Revision: 940898

URL: http://svn.apache.org/viewvc?rev=940898&view=rev
Log:
Add a function for deleting unreferenced pristine text files.

* subversion/libsvn_wc/wc_db.c,
  subversion/libsvn_wc/wc_db.h
  (svn_wc__db_pristine_remove): New function.

* subversion/libsvn_wc/wc-queries.sql
  (STMT_SELECT_ANY_PRISTINE_REFERENCE): New query.

* subversion/tests/libsvn_wc/pristine-store-test.c
  (pristine_write_read): Also trivially test a removal.

Modified:
    subversion/trunk/subversion/libsvn_wc/wc-queries.sql
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/libsvn_wc/wc_db.h
    subversion/trunk/subversion/tests/libsvn_wc/pristine-store-test.c

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=940898&r1=940897&r2=940898&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Tue May  4 15:14:13 2010
@@ -289,6 +289,23 @@ SELECT md5_checksum
 FROM pristine
 WHERE checksum = ?1
 
+-- STMT_SELECT_ANY_PRISTINE_REFERENCE
+SELECT 1 FROM base_node
+  WHERE checksum = ?1 OR checksum = ?2
+UNION ALL
+SELECT 1 FROM working_node
+  WHERE checksum = ?1 OR checksum = ?2
+UNION ALL
+SELECT 1 FROM actual_node
+  WHERE older_checksum = ?1 OR older_checksum = ?2
+    OR  left_checksum  = ?1 OR left_checksum  = ?2
+    OR  right_checksum = ?1 OR right_checksum = ?2
+LIMIT 1
+
+-- STMT_DELETE_PRISTINE
+DELETE FROM PRISTINE
+WHERE checksum = ?1
+
 -- STMT_SELECT_ACTUAL_CONFLICT_VICTIMS
 SELECT local_relpath
 FROM actual_node

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=940898&r1=940897&r2=940898&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue May  4 15:14:13 2010
@@ -2560,6 +2560,75 @@ svn_wc__db_pristine_install(svn_wc__db_t
 
 
 svn_error_t *
+svn_wc__db_pristine_remove(svn_wc__db_t *db,
+                           const char *wri_abspath,
+                           const svn_checksum_t *sha1_checksum,
+                           apr_pool_t *scratch_pool)
+{
+  svn_wc__db_pdh_t *pdh;
+  const char *local_relpath;
+  const char *pristine_abspath;
+  svn_error_t *err;
+  const svn_checksum_t *md5_checksum;
+  svn_boolean_t is_referenced = FALSE;
+
+  SVN_ERR_ASSERT(svn_dirent_is_absolute(wri_abspath));
+  SVN_ERR_ASSERT(sha1_checksum->kind == svn_checksum_sha1);
+
+  SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, wri_abspath,
+                              svn_sqlite__mode_readwrite,
+                              scratch_pool, scratch_pool));
+  VERIFY_USABLE_PDH(pdh);
+
+  err = get_pristine_fname(&pristine_abspath, pdh, sha1_checksum,
+#ifndef SVN__SKIP_SUBDIR
+                           TRUE /* create_subdir */,
+#endif
+                           scratch_pool, scratch_pool);
+  SVN_ERR(err);
+
+  /* Find whether the SHA-1 (or the MD-5) is referenced; set IS_REFERENCED. */
+  {
+    svn_sqlite__stmt_t *stmt;
+
+    /* ### Transitional: look for references to its MD-5 as well. */
+    SVN_ERR(svn_wc__db_pristine_get_md5(&md5_checksum, db, wri_abspath,
+                                        sha1_checksum, scratch_pool, scratch_pool));
+
+    SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb,
+                                      STMT_SELECT_ANY_PRISTINE_REFERENCE));
+    SVN_ERR(svn_sqlite__bind_checksum(stmt, 1, sha1_checksum, scratch_pool));
+    SVN_ERR(svn_sqlite__bind_checksum(stmt, 2, md5_checksum, scratch_pool));
+    SVN_ERR(svn_sqlite__step(&is_referenced, stmt));
+
+    SVN_ERR(svn_sqlite__reset(stmt));
+  }
+
+  /* If not referenced, remove first the PRISTINE table row, then the file. */
+  if (! is_referenced)
+    {
+      svn_sqlite__stmt_t *stmt;
+
+      /* Remove the DB row. */
+      SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb,
+                                        STMT_DELETE_PRISTINE));
+      SVN_ERR(svn_sqlite__bind_checksum(stmt, 1, sha1_checksum, scratch_pool));
+      SVN_ERR(svn_sqlite__update(NULL, stmt));
+
+      /* Remove the file */
+      SVN_ERR(svn_io_remove_file2(pristine_abspath, TRUE, scratch_pool));
+    SVN_DBG(("Was referenced: '%s'\n",
+           svn_checksum_to_cstring_display(md5_checksum, scratch_pool)));
+    }
+  else
+    SVN_DBG(("Not referenced: '%s'\n",
+           svn_checksum_to_cstring_display(md5_checksum, scratch_pool)));
+
+  return SVN_NO_ERROR;
+}
+
+
+svn_error_t *
 svn_wc__db_pristine_check(svn_boolean_t *present,
                           svn_wc__db_t *db,
                           const char *wri_abspath,

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=940898&r1=940897&r2=940898&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Tue May  4 15:14:13 2010
@@ -866,6 +866,16 @@ svn_wc__db_pristine_get_md5(const svn_ch
                             apr_pool_t *scratch_pool);
 
 
+/* Remove the pristine text with SHA-1 checksum SHA1_CHECKSUM from the
+ * pristine store, iff it is not referenced by any of the (other) WC DB
+ * tables. */
+svn_error_t *
+svn_wc__db_pristine_remove(svn_wc__db_t *db,
+                           const char *wri_abspath,
+                           const svn_checksum_t *sha1_checksum,
+                           apr_pool_t *scratch_pool);
+
+
 /* ### check for presence, according to the given mode (on how hard we
    ### should examine things)
 */

Modified: subversion/trunk/subversion/tests/libsvn_wc/pristine-store-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/pristine-store-test.c?rev=940898&r1=940897&r2=940898&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/pristine-store-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/pristine-store-test.c Tue May  4 15:14:13 2010
@@ -183,6 +183,19 @@ pristine_write_read(const svn_test_opts_
     SVN_TEST_ASSERT(same);
   }
 
+  /* Trivially test the "remove if unreferenced" API: it's not referenced
+     so we should be able to remove it. */
+  {
+    svn_error_t *err;
+    svn_stream_t *data_read_back;
+
+    SVN_ERR(svn_wc__db_pristine_remove(db, wc_abspath, data_sha1, pool));
+    err = svn_wc__db_pristine_read(&data_read_back, db, wc_abspath,
+                                   data_sha1, pool, pool);
+    SVN_TEST_ASSERT(err != NULL);
+    svn_error_clear(err);
+  }
+
   return SVN_NO_ERROR;
 }
 



RE: svn commit: r940898 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/pristine-store-test.c

Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2010-05-05 at 12:30 +0200, Bert Huijben wrote:
> > Log:
> > Add a function for deleting unreferenced pristine text files.
> > 
> > * subversion/libsvn_wc/wc_db.c,
> >   subversion/libsvn_wc/wc_db.h
> >   (svn_wc__db_pristine_remove): New function.
> > 
> > * subversion/libsvn_wc/wc-queries.sql
> >   (STMT_SELECT_ANY_PRISTINE_REFERENCE): New query.
> 
> You also added STMT_DELETE_PRISTINE in this patch.

Thanks.  I've fixed the log message.

- Julian



RE: svn commit: r940898 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/pristine-store-test.c

Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2010-05-05 at 12:30 +0200, Bert Huijben wrote:
> > Log:
> > Add a function for deleting unreferenced pristine text files.
> > 
> > * subversion/libsvn_wc/wc_db.c,
> >   subversion/libsvn_wc/wc_db.h
> >   (svn_wc__db_pristine_remove): New function.
> > 
> > * subversion/libsvn_wc/wc-queries.sql
> >   (STMT_SELECT_ANY_PRISTINE_REFERENCE): New query.
> 
> You also added STMT_DELETE_PRISTINE in this patch.

Thanks.  I've fixed the log message.

- Julian


RE: svn commit: r940898 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/pristine-store-test.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: julianfoad@apache.org [mailto:julianfoad@apache.org]
> Sent: dinsdag 4 mei 2010 17:14
> To: commits@subversion.apache.org
> Subject: svn commit: r940898 - in /subversion/trunk/subversion:
> libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h
> tests/libsvn_wc/pristine-store-test.c
> 
> Author: julianfoad
> Date: Tue May  4 15:14:13 2010
> New Revision: 940898
> 
> URL: http://svn.apache.org/viewvc?rev=940898&view=rev
> Log:
> Add a function for deleting unreferenced pristine text files.
> 
> * subversion/libsvn_wc/wc_db.c,
>   subversion/libsvn_wc/wc_db.h
>   (svn_wc__db_pristine_remove): New function.
> 
> * subversion/libsvn_wc/wc-queries.sql
>   (STMT_SELECT_ANY_PRISTINE_REFERENCE): New query.

You also added STMT_DELETE_PRISTINE in this patch.

	Bert


RE: svn commit: r940898 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/pristine-store-test.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: julianfoad@apache.org [mailto:julianfoad@apache.org]
> Sent: dinsdag 4 mei 2010 17:14
> To: commits@subversion.apache.org
> Subject: svn commit: r940898 - in /subversion/trunk/subversion:
> libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h
> tests/libsvn_wc/pristine-store-test.c
> 
> Author: julianfoad
> Date: Tue May  4 15:14:13 2010
> New Revision: 940898
> 
> URL: http://svn.apache.org/viewvc?rev=940898&view=rev
> Log:
> Add a function for deleting unreferenced pristine text files.
> 
> * subversion/libsvn_wc/wc_db.c,
>   subversion/libsvn_wc/wc_db.h
>   (svn_wc__db_pristine_remove): New function.
> 
> * subversion/libsvn_wc/wc-queries.sql
>   (STMT_SELECT_ANY_PRISTINE_REFERENCE): New query.

You also added STMT_DELETE_PRISTINE in this patch.

	Bert

Re: svn commit: r940898 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/pristine-store-test.c

Posted by Greg Stein <gs...@gmail.com>.
On Wed, May 5, 2010 at 05:31, Julian Foad <ju...@wandisco.com> wrote:
>...
> All the above: r941212.

Cool.

>...
>> > +/* Remove the pristine text with SHA-1 checksum SHA1_CHECKSUM from the
>> > + * pristine store, iff it is not referenced by any of the (other) WC DB
>> > + * tables. */
>>
>> Also remember that we don't want to remove pristines why
>> administrative locks (WCLOCK) are present. That may signify a commit
>> is occurring and needed pristines are transiently living in the
>> pristine storage.
>
> I need to think on this some more and learn about WCLOCK.
>
> If I'm using "pristine_remove" in a general garbage-collection manner
> (outside of any code path that installs pristine texts), I would want to
> bail if there are any entries in the WCLOCK table.

Yes.

I think. :-P ... if those locks are held by "this process", then
sure... something may be holding the SHA-1 keys and will be doing
something with those pristines. If the locks are *not* held by this
process, then something else is doing some work (definitely skip
GC'ing!) or a prior run failed in some way and there are stale locks
remaining, pending an 'svn cleanup'. Presumably, there will also be
some work in the queue.

> If I'm using "pristine_remove" within a commit or update code path
> (where I am also installing new text bases, but I can ensure that the
> one I'm trying to remove is not one that I'm in the process of
> installing), then I would want to bail if there are any locks in the
> WCLOCK table *other than* those locks that are in WC_CTX.

If the pristine is not in the table, and there are no Other Locks,
then yes. I think this sounds right. I believe the design should be
"pristines that need to live beyond locks require a reference from a
table". Thus, if your process does not have a private reference to a
pristine (known via code path), and no *other* process has one (by
virtue of no WCLOCK), then you're free to GC pristines.

Cheers,
-g

Re: svn commit: r940898 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/pristine-store-test.c

Posted by Julian Foad <ju...@wandisco.com>.
Greg Stein wrote:
> On Tue, May 4, 2010 at 11:14,  <ju...@apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue May  4 15:14:13 2010
> > @@ -2560,6 +2560,75 @@ svn_wc__db_pristine_install(svn_wc__db_t
> >
> >
> >  svn_error_t *
> > +svn_wc__db_pristine_remove(svn_wc__db_t *db,
> > +                           const char *wri_abspath,
> > +                           const svn_checksum_t *sha1_checksum,
> > +                           apr_pool_t *scratch_pool)
> > +{
> > +  svn_wc__db_pdh_t *pdh;
> > +  const char *local_relpath;
> > +  const char *pristine_abspath;
> > +  svn_error_t *err;
> > +  const svn_checksum_t *md5_checksum;
> > +  svn_boolean_t is_referenced = FALSE;
> 
> Why initialize this? It will be unconditionally set below.

Oops, fixed.

> > +
> > +  SVN_ERR_ASSERT(svn_dirent_is_absolute(wri_abspath));
> > +  SVN_ERR_ASSERT(sha1_checksum->kind == svn_checksum_sha1);
> > +
> > +  SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, wri_abspath,
> > +                              svn_sqlite__mode_readwrite,
> > +                              scratch_pool, scratch_pool));
> > +  VERIFY_USABLE_PDH(pdh);
> > +
> > +  err = get_pristine_fname(&pristine_abspath, pdh, sha1_checksum,
> > +#ifndef SVN__SKIP_SUBDIR
> > +                           TRUE /* create_subdir */,
> > +#endif
> > +                           scratch_pool, scratch_pool);
> > +  SVN_ERR(err);
> 
> Why compute the path so early? This won't be needed unless/until you
> actually determine the file should be removed.

Thanks, fixed.

[...]
> > +    SVN_DBG(("Was referenced: '%s'\n",
> > +           svn_checksum_to_cstring_display(md5_checksum, scratch_pool)));
> 
> Here...
> 
> > +    }
> > +  else
> > +    SVN_DBG(("Not referenced: '%s'\n",
> > +           svn_checksum_to_cstring_display(md5_checksum, scratch_pool)));
> 
> ... and here. SVN_DBG cannot be left in the committed source code. The
> macro does not exist in release builds.

Oops, fixed.

All the above: r941212.

> >...
> > +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Tue May  4 15:14:13 2010
> > @@ -866,6 +866,16 @@ svn_wc__db_pristine_get_md5(const svn_ch
> >                             apr_pool_t *scratch_pool);
> >
> >
> > +/* Remove the pristine text with SHA-1 checksum SHA1_CHECKSUM from the
> > + * pristine store, iff it is not referenced by any of the (other) WC DB
> > + * tables. */
> 
> Also remember that we don't want to remove pristines why
> administrative locks (WCLOCK) are present. That may signify a commit
> is occurring and needed pristines are transiently living in the
> pristine storage.

I need to think on this some more and learn about WCLOCK.

If I'm using "pristine_remove" in a general garbage-collection manner
(outside of any code path that installs pristine texts), I would want to
bail if there are any entries in the WCLOCK table.

If I'm using "pristine_remove" within a commit or update code path
(where I am also installing new text bases, but I can ensure that the
one I'm trying to remove is not one that I'm in the process of
installing), then I would want to bail if there are any locks in the
WCLOCK table *other than* those locks that are in WC_CTX.

Does that sound right?

- Julian


Re: svn commit: r940898 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/pristine-store-test.c

Posted by Greg Stein <gs...@gmail.com>.
On Tue, May 4, 2010 at 11:14,  <ju...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue May  4 15:14:13 2010
> @@ -2560,6 +2560,75 @@ svn_wc__db_pristine_install(svn_wc__db_t
>
>
>  svn_error_t *
> +svn_wc__db_pristine_remove(svn_wc__db_t *db,
> +                           const char *wri_abspath,
> +                           const svn_checksum_t *sha1_checksum,
> +                           apr_pool_t *scratch_pool)
> +{
> +  svn_wc__db_pdh_t *pdh;
> +  const char *local_relpath;
> +  const char *pristine_abspath;
> +  svn_error_t *err;
> +  const svn_checksum_t *md5_checksum;
> +  svn_boolean_t is_referenced = FALSE;

Why initialize this? It will be unconditionally set below.

> +
> +  SVN_ERR_ASSERT(svn_dirent_is_absolute(wri_abspath));
> +  SVN_ERR_ASSERT(sha1_checksum->kind == svn_checksum_sha1);
> +
> +  SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, wri_abspath,
> +                              svn_sqlite__mode_readwrite,
> +                              scratch_pool, scratch_pool));
> +  VERIFY_USABLE_PDH(pdh);
> +
> +  err = get_pristine_fname(&pristine_abspath, pdh, sha1_checksum,
> +#ifndef SVN__SKIP_SUBDIR
> +                           TRUE /* create_subdir */,
> +#endif
> +                           scratch_pool, scratch_pool);
> +  SVN_ERR(err);

Why compute the path so early? This won't be needed unless/until you
actually determine the file should be removed.

> +
> +  /* Find whether the SHA-1 (or the MD-5) is referenced; set IS_REFERENCED. */
> +  {
> +    svn_sqlite__stmt_t *stmt;
> +
> +    /* ### Transitional: look for references to its MD-5 as well. */
> +    SVN_ERR(svn_wc__db_pristine_get_md5(&md5_checksum, db, wri_abspath,
> +                                        sha1_checksum, scratch_pool, scratch_pool));
> +
> +    SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb,
> +                                      STMT_SELECT_ANY_PRISTINE_REFERENCE));
> +    SVN_ERR(svn_sqlite__bind_checksum(stmt, 1, sha1_checksum, scratch_pool));
> +    SVN_ERR(svn_sqlite__bind_checksum(stmt, 2, md5_checksum, scratch_pool));
> +    SVN_ERR(svn_sqlite__step(&is_referenced, stmt));
> +
> +    SVN_ERR(svn_sqlite__reset(stmt));
> +  }
> +
> +  /* If not referenced, remove first the PRISTINE table row, then the file. */
> +  if (! is_referenced)
> +    {
> +      svn_sqlite__stmt_t *stmt;
> +
> +      /* Remove the DB row. */
> +      SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb,
> +                                        STMT_DELETE_PRISTINE));
> +      SVN_ERR(svn_sqlite__bind_checksum(stmt, 1, sha1_checksum, scratch_pool));
> +      SVN_ERR(svn_sqlite__update(NULL, stmt));
> +
> +      /* Remove the file */
> +      SVN_ERR(svn_io_remove_file2(pristine_abspath, TRUE, scratch_pool));
> +    SVN_DBG(("Was referenced: '%s'\n",
> +           svn_checksum_to_cstring_display(md5_checksum, scratch_pool)));

Here...

> +    }
> +  else
> +    SVN_DBG(("Not referenced: '%s'\n",
> +           svn_checksum_to_cstring_display(md5_checksum, scratch_pool)));

... and here. SVN_DBG cannot be left in the committed source code. The
macro does not exist in release builds.

>...
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Tue May  4 15:14:13 2010
> @@ -866,6 +866,16 @@ svn_wc__db_pristine_get_md5(const svn_ch
>                             apr_pool_t *scratch_pool);
>
>
> +/* Remove the pristine text with SHA-1 checksum SHA1_CHECKSUM from the
> + * pristine store, iff it is not referenced by any of the (other) WC DB
> + * tables. */

Also remember that we don't want to remove pristines why
administrative locks (WCLOCK) are present. That may signify a commit
is occurring and needed pristines are transiently living in the
pristine storage.

>...

Cheers,
-g