subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stef...@apache.org
Subject svn commit: r1759124 - in /subversion/trunk/subversion: libsvn_fs_fs/fs.c libsvn_fs_fs/pack.c libsvn_fs_fs/pack.h libsvn_fs_fs/tree.c tests/libsvn_fs_fs/fs-fs-pack-test.c
Date Sat, 03 Sep 2016 19:04:38 GMT
Author: stefan2
Date: Sat Sep  3 19:04:37 2016
New Revision: 1759124

URL: http://svn.apache.org/viewvc?rev=1759124&view=rev
Log:
Make FSFS packing with restricted memory testable and add a test for it.

We basically expose the MAX_MEM parameter used internally at our private
FSFS API level and call that in the test.  We could also expose it in the
public FS API but that doesn't seem to be necessary ATM.

* subversion/libsvn_fs_fs/pack.h
  (svn_fs_fs__pack): Add optional MAX_MEM parameter.

* subversion/libsvn_fs_fs/pack.c
  (pack_baton,
   pack_shard): Take the MAX_MEM parameter from the baton now.
  (svn_fs_fs__pack): Pass the optional parameter on to the baton.

* subversion/libsvn_fs_fs/fs.c
  (fs_pack): Update caller.

* subversion/libsvn_fs_fs/tree.c
  (svn_fs_fs__commit_txn): Same.

* subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
  (create_non_packed_filesystem): Extract this new utility function out
                                  from ...
  (create_packed_filesystem): ... this.
  (pack_with_limited_memory): New test.
  (test_funcs): Register the new test.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/fs.c
    subversion/trunk/subversion/libsvn_fs_fs/pack.c
    subversion/trunk/subversion/libsvn_fs_fs/pack.h
    subversion/trunk/subversion/libsvn_fs_fs/tree.c
    subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.c?rev=1759124&r1=1759123&r2=1759124&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs.c Sat Sep  3 19:04:37 2016
@@ -475,7 +475,7 @@ fs_pack(svn_fs_t *fs,
         apr_pool_t *common_pool)
 {
   SVN_ERR(fs_open(fs, path, common_pool_lock, pool, common_pool));
-  return svn_fs_fs__pack(fs, notify_func, notify_baton,
+  return svn_fs_fs__pack(fs, 0, notify_func, notify_baton,
                          cancel_func, cancel_baton, pool);
 }
 

Modified: subversion/trunk/subversion/libsvn_fs_fs/pack.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/pack.c?rev=1759124&r1=1759123&r2=1759124&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/pack.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/pack.c Sat Sep  3 19:04:37 2016
@@ -1840,6 +1840,7 @@ struct pack_baton
   void *notify_baton;
   svn_cancel_func_t cancel_func;
   void *cancel_baton;
+  size_t max_mem;
 
   /* Additional entries valid when entering pack_shard(). */
   const char *revs_dir;
@@ -1964,7 +1965,7 @@ pack_shard(struct pack_baton *baton,
   /* pack the revision content */
   SVN_ERR(pack_rev_shard(baton->fs, rev_pack_file_dir, baton->rev_shard_path,
                          baton->shard, ffd->max_files_per_dir,
-                         DEFAULT_MAX_MEM, ffd->flush_to_disk,
+                         baton->max_mem, ffd->flush_to_disk,
                          baton->cancel_func, baton->cancel_baton, pool));
 
   /* For newer repo formats, we only acquired the pack lock so far.
@@ -2074,6 +2075,7 @@ pack_body(void *baton,
 
 svn_error_t *
 svn_fs_fs__pack(svn_fs_t *fs,
+                apr_size_t max_mem,
                 svn_fs_pack_notify_t notify_func,
                 void *notify_baton,
                 svn_cancel_func_t cancel_func,
@@ -2119,6 +2121,7 @@ svn_fs_fs__pack(svn_fs_t *fs,
   pb.notify_baton = notify_baton;
   pb.cancel_func = cancel_func;
   pb.cancel_baton = cancel_baton;
+  pb.max_mem = max_mem ? max_mem : DEFAULT_MAX_MEM;
 
   if (ffd->format >= SVN_FS_FS__MIN_PACK_LOCK_FORMAT)
     {

Modified: subversion/trunk/subversion/libsvn_fs_fs/pack.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/pack.h?rev=1759124&r1=1759123&r2=1759124&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/pack.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/pack.h Sat Sep  3 19:04:37 2016
@@ -26,13 +26,19 @@
 #include "fs.h"
 
 /* Possibly pack the repository at PATH.  This just take full shards, and
-   combines all the revision files into a single one, with a manifest header.
+   combines all the revision files into a single one, with a manifest header
+   when required by the repository format.
+
+   MAX_MEM limits the size of in-memory data structures needed for reordering
+   items in format 7 repositories.  0 means use the built-in default.
+
    If given, NOTIFY_FUNC will be called with NOTIFY_BATON to report progress.
    Use optional CANCEL_FUNC/CANCEL_BATON for cancellation support.
 
    Existing filesystem references need not change.  */
 svn_error_t *
 svn_fs_fs__pack(svn_fs_t *fs,
+                apr_size_t max_mem,
                 svn_fs_pack_notify_t notify_func,
                 void *notify_baton,
                 svn_cancel_func_t cancel_func,

Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1759124&r1=1759123&r2=1759124&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Sat Sep  3 19:04:37 2016
@@ -2314,7 +2314,7 @@ svn_fs_fs__commit_txn(const char **confl
 
   if (ffd->pack_after_commit)
     {
-      SVN_ERR(svn_fs_fs__pack(fs, NULL, NULL, NULL, NULL, pool));
+      SVN_ERR(svn_fs_fs__pack(fs, 0, NULL, NULL, NULL, NULL, pool));
     }
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c?rev=1759124&r1=1759123&r2=1759124&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c Sat Sep  3 19:04:37 2016
@@ -29,6 +29,7 @@
 #include "../../libsvn_fs_fs/fs.h"
 #include "../../libsvn_fs_fs/fs_fs.h"
 #include "../../libsvn_fs_fs/low_level.h"
+#include "../../libsvn_fs_fs/pack.h"
 #include "../../libsvn_fs_fs/util.h"
 
 #include "svn_hash.h"
@@ -101,17 +102,16 @@ pack_notify(void *baton,
 
 #define R1_LOG_MSG "Let's serf"
 
-/* Create a packed filesystem in DIR.  Set the shard size to
-   SHARD_SIZE and create NUM_REVS number of revisions (in addition to
-   r0).  Use POOL for allocations.  After this function successfully
-   completes, the filesystem's youngest revision number will be the
-   same as NUM_REVS.  */
-static svn_error_t *
-create_packed_filesystem(const char *dir,
-                         const svn_test_opts_t *opts,
-                         svn_revnum_t num_revs,
-                         int shard_size,
-                         apr_pool_t *pool)
+/* Create a filesystem in DIR.  Set the shard size to SHARD_SIZE and create
+   NUM_REVS number of revisions (in addition to r0).  Use POOL for
+   allocations.  After this function successfully completes, the filesystem's
+   youngest revision number will be NUM_REVS.  */
+static svn_error_t *
+create_non_packed_filesystem(const char *dir,
+                             const svn_test_opts_t *opts,
+                             svn_revnum_t num_revs,
+                             int shard_size,
+                             apr_pool_t *pool)
 {
   svn_fs_t *fs;
   svn_fs_txn_t *txn;
@@ -119,7 +119,6 @@ create_packed_filesystem(const char *dir
   const char *conflict;
   svn_revnum_t after_rev;
   apr_pool_t *subpool = svn_pool_create(pool);
-  struct pack_notify_baton pnb;
   apr_pool_t *iterpool;
   apr_hash_t *fs_config;
 
@@ -166,6 +165,28 @@ create_packed_filesystem(const char *dir
   svn_pool_destroy(iterpool);
   svn_pool_destroy(subpool);
 
+  /* Done */
+  return SVN_NO_ERROR;
+}
+
+/* Create a packed filesystem in DIR.  Set the shard size to
+   SHARD_SIZE and create NUM_REVS number of revisions (in addition to
+   r0).  Use POOL for allocations.  After this function successfully
+   completes, the filesystem's youngest revision number will be the
+   same as NUM_REVS.  */
+static svn_error_t *
+create_packed_filesystem(const char *dir,
+                         const svn_test_opts_t *opts,
+                         svn_revnum_t num_revs,
+                         int shard_size,
+                         apr_pool_t *pool)
+{
+  struct pack_notify_baton pnb;
+
+  /* Create the repo and fill it. */
+  SVN_ERR(create_non_packed_filesystem(dir, opts, num_revs, shard_size,
+                                       pool));
+
   /* Now pack the FS */
   pnb.expected_shard = 0;
   pnb.expected_action = svn_fs_pack_notify_start;
@@ -1740,6 +1761,56 @@ compare_0_length_rep(const svn_test_opts
 
 #undef REPO_NAME
 
+/* ------------------------------------------------------------------------ */
+/* Verify that the format 7 pack logic works even if we can't fit all index
+   metadata into memory. */
+#define REPO_NAME "test-repo-pack-with-limited-memory"
+#define SHARD_SIZE 4
+#define MAX_REV (2 * SHARD_SIZE - 1)
+static svn_error_t *
+pack_with_limited_memory(const svn_test_opts_t *opts,
+                         apr_pool_t *pool)
+{
+  apr_size_t max_mem;
+  apr_pool_t *iterpool = svn_pool_create(pool);
+
+  /* Bail (with success) on known-untestable scenarios */
+  if (opts->server_minor_version && (opts->server_minor_version < 9))
+    return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL,
+                            "pre-1.9 SVN doesn't support reordering packs");
+
+  /* Run with an increasing memory allowance such that we cover all
+     splitting scenarios. */
+  for (max_mem = 350; max_mem < 8000; max_mem += max_mem / 2)
+    {
+      const char *dir;
+      svn_fs_t *fs;
+
+      svn_pool_clear(iterpool);
+
+      /* Create a filesystem. */
+      dir = apr_psprintf(iterpool, "%s-%d", REPO_NAME, (int)max_mem);
+      SVN_ERR(create_non_packed_filesystem(dir, opts, MAX_REV, SHARD_SIZE,
+                                           iterpool));
+
+      /* Pack it with a narrow memory budget. */
+      SVN_ERR(svn_fs_open2(&fs, dir, NULL, iterpool, iterpool));
+      SVN_ERR(svn_fs_fs__pack(fs, max_mem, NULL, NULL, NULL, NULL,
+                              iterpool));
+
+      /* To be sure: Verify that we didn't break the repo. */
+      SVN_ERR(svn_fs_verify(dir, NULL, 0, MAX_REV, NULL, NULL, NULL, NULL,
+                            iterpool));
+    }
+
+  svn_pool_destroy(iterpool);
+
+  return SVN_NO_ERROR;
+}
+#undef REPO_NAME
+#undef MAX_REV
+#undef SHARD_SIZE
+
 
 /* The test table.  */
 
@@ -1790,6 +1861,8 @@ static struct svn_test_descriptor_t test
                        "delta chains starting with PLAIN, issue #4577"),
     SVN_TEST_OPTS_PASS(compare_0_length_rep,
                        "compare empty PLAIN and non-existent reps"),
+    SVN_TEST_OPTS_PASS(pack_with_limited_memory,
+                       "pack with limited memory for metadata"),
     SVN_TEST_NULL
   };
 



Mime
View raw message