You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2016/03/23 18:39:37 UTC

svn commit: r1736357 - in /subversion/trunk: subversion/include/ subversion/libsvn_fs_fs/ subversion/svnadmin/ subversion/tests/cmdline/ tools/client-side/

Author: kotkov
Date: Wed Mar 23 17:39:36 2016
New Revision: 1736357

URL: http://svn.apache.org/viewvc?rev=1736357&view=rev
Log:
Introduce `--no-flush-to-disk' option for `svnadmin load'.

The option can be used to to dramatically speed up the load process when
there's no need to ensure that the resulting data survives a system crash
or power loss — e.g., when loading a dump into a fresh new repository.

This is one of the ideas in http://svn.haxx.se/dev/archive-2015-09/0187.shtml
(Subject: "Whiteboard -- topics list on the white board").

* subversion/include/svn_fs.h
  (SVN_FS_CONFIG_NO_FLUSH_TO_DISK): New option.

* subversion/libsvn_fs_fs/fs.h
  (fs_fs_data_t): Add `flush_to_disk' boolean field.

* subversion/libsvn_fs_fs/fs.с
  (initialize_fs_struct): Initialize the new field.

* subversion/libsvn_fs_fs/fs_fs.c
  (read_global_config): Set the new field based on what's in fs->config.

* subversion/libsvn_fs_fs/util.h
  (svn_fs_fs__move_into_place): Accept a new `flush_to_disk' argument.

* subversion/libsvn_fs_fs/util.c
  (svn_fs_fs__move_into_place): Make the flush optional based on the
   new argument.

* subversion/libsvn_fs_fs/transaction.c
  (get_and_increment_txn_key_body): Don't flush to disk if that's allowed.
  (write_final_revprop): Accept a new `flush_to_disk' argument.  Make the
   flush optional based on the new argument.
  (commit_body): Don't flush to disk if that's allowed.  Adjust calls to
   write_final_revprop() and svn_fs_fs__move_into_place().

* subversion/libsvn_fs_fs/revprops.c
  (switch_to_new_revprop): Adjust the call to svn_fs_fs__move_into_place().
   Keep the existing behavior and always flush to disk.

* subversion/svnadmin/svnadmin.c
  (svnadmin__no_flush_to_disk): New enum value.
  (options_table): Define --no-flush-to-disk option.
  (cmd_table): Allow `load' to accept --no-flush-to-disk.
  (svnadmin_opt_state): Add `no_flush_to_disk' member.
  (open_repos): Move below the definition of svnadmin_opt_state.  Accept
   an svnadmin_opt_state structure as one of the arguments and initialize
   the new SVN_FS_CONFIG_NO_FLUSH_TO_DISK option based on it.
  (subcommand_crashtest, subcommand_deltify, subcommand_dump,
   subcommand_dump_revprops, subcommand_load, subcommand_load_revprops,
   subcommand_lstxns, subcommand_recover, subcommand_rmtxns, set_revprop,
   subcommand_setuuid, subcommand_pack, subcommand_verify, subcommand_info,
   subcommand_lock, subcommand_lslocks, subcommand_rmlocks,
   subcommand_unlock): Adjust these callers of open_repos().
  (main): Handle --no-flush-to-disk option.

* subversion/tests/cmdline/svnadmin_tests.py
  (load_no_flush_to_disk): New test.
  (test_list): Add reference to new test.

* tools/client-side/bash_completion
  (_svnadmin): Add new option to `load'.

Modified:
    subversion/trunk/subversion/include/svn_fs.h
    subversion/trunk/subversion/libsvn_fs_fs/fs.c
    subversion/trunk/subversion/libsvn_fs_fs/fs.h
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
    subversion/trunk/subversion/libsvn_fs_fs/revprops.c
    subversion/trunk/subversion/libsvn_fs_fs/transaction.c
    subversion/trunk/subversion/libsvn_fs_fs/util.c
    subversion/trunk/subversion/libsvn_fs_fs/util.h
    subversion/trunk/subversion/svnadmin/svnadmin.c
    subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
    subversion/trunk/tools/client-side/bash_completion

Modified: subversion/trunk/subversion/include/svn_fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_fs.h?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_fs.h (original)
+++ subversion/trunk/subversion/include/svn_fs.h Wed Mar 23 17:39:36 2016
@@ -213,6 +213,20 @@ typedef struct svn_fs_t svn_fs_t;
  * @since New in 1.9.
  */
 #define SVN_FS_CONFIG_COMPATIBLE_VERSION        "compatible-version"
+
+/** Specifies whether the filesystem should be forcing a physical write of
+ * the data to disk.  Enabling the option allows the filesystem to return
+ * from the API calls without forcing the write to disk.  If this option
+ * is disabled, the changes are always written to disk.
+ *
+ * @note Avoiding the forced write to disk usually is more efficient, but
+ * doesn't guarantee data integrity after a system crash or power failure
+ * and should be used with caution.
+ *
+ * @since New in 1.10.
+ */
+#define SVN_FS_CONFIG_NO_FLUSH_TO_DISK          "no-flush-to-disk"
+
 /** @} */
 
 

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.c?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs.c Wed Mar 23 17:39:36 2016
@@ -292,6 +292,7 @@ initialize_fs_struct(svn_fs_t *fs)
   fs_fs_data_t *ffd = apr_pcalloc(fs->pool, sizeof(*ffd));
   ffd->use_log_addressing = FALSE;
   ffd->revprop_prefix = 0;
+  ffd->flush_to_disk = FALSE;
 
   fs->vtable = &fs_vtable;
   fs->fsap_data = ffd;

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.h?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs.h Wed Mar 23 17:39:36 2016
@@ -476,6 +476,9 @@ typedef struct fs_fs_data_t
      or dump / load cycles). */
   const char *instance_id;
 
+  /* Ensure that all filesystem changes are written to disk. */
+  svn_boolean_t flush_to_disk;
+
   /* Pointer to svn_fs_open. */
   svn_error_t *(*svn_fs_open_)(svn_fs_t **, const char *, apr_hash_t *,
                                apr_pool_t *, apr_pool_t *);

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Mar 23 17:39:36 2016
@@ -1040,6 +1040,10 @@ read_global_config(svn_fs_t *fs)
   else
     ffd->use_block_read = FALSE;
 
+  ffd->flush_to_disk = !svn_hash__get_bool(fs->config,
+                                           SVN_FS_CONFIG_NO_FLUSH_TO_DISK,
+                                           FALSE);
+
   /* Ignore the user-specified larger block size if we don't use block-read.
      Defaulting to 4k gives us the same access granularity in format 7 as in
      older formats. */

Modified: subversion/trunk/subversion/libsvn_fs_fs/revprops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/revprops.c?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/revprops.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/revprops.c Wed Mar 23 17:39:36 2016
@@ -776,7 +776,7 @@ switch_to_new_revprop(svn_fs_t *fs,
                       apr_pool_t *pool)
 {
   SVN_ERR(svn_fs_fs__move_into_place(tmp_path, final_path, perms_reference,
-                                     pool));
+                                     TRUE, pool));
 
   /* Clean up temporary files, if necessary. */
   if (files_to_delete)

Modified: subversion/trunk/subversion/libsvn_fs_fs/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/transaction.c?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Wed Mar 23 17:39:36 2016
@@ -997,6 +997,7 @@ static svn_error_t *
 get_and_increment_txn_key_body(void *baton, apr_pool_t *pool)
 {
   struct get_and_increment_txn_key_baton *cb = baton;
+  fs_fs_data_t *ffd = cb->fs->fsap_data;
   const char *txn_current_filename
     = svn_fs_fs__path_txn_current(cb->fs, pool);
   char new_id_str[SVN_INT64_BUFFER_SIZE + 1]; /* add space for a newline */
@@ -1017,7 +1018,7 @@ get_and_increment_txn_key_body(void *bat
   SVN_ERR(svn_io_write_atomic2(txn_current_filename, new_id_str,
                                line_length + 1,
                                txn_current_filename /* copy_perms path */,
-                               TRUE, pool));
+                               ffd->flush_to_disk, pool));
 
   return SVN_NO_ERROR;
 }
@@ -3440,6 +3441,7 @@ static svn_error_t *
 write_final_revprop(const char *path,
                     const char *perms_reference,
                     svn_fs_txn_t *txn,
+                    svn_boolean_t flush_to_disk,
                     apr_pool_t *pool)
 {
   apr_hash_t *txnprops;
@@ -3479,7 +3481,8 @@ write_final_revprop(const char *path,
   SVN_ERR(svn_hash_write2(txnprops, stream, SVN_HASH_TERMINATOR, pool));
   SVN_ERR(svn_stream_close(stream));
 
-  SVN_ERR(svn_io_file_flush_to_disk(revprop_file, pool));
+  if (flush_to_disk)
+    SVN_ERR(svn_io_file_flush_to_disk(revprop_file, pool));
   SVN_ERR(svn_io_file_close(revprop_file, pool));
 
   SVN_ERR(svn_io_copy_perms(perms_reference, path, pool));
@@ -3676,7 +3679,8 @@ commit_body(void *baton, apr_pool_t *poo
                                      NULL, pool));
     }
 
-  SVN_ERR(svn_io_file_flush_to_disk(proto_file, pool));
+  if (ffd->flush_to_disk)
+    SVN_ERR(svn_io_file_flush_to_disk(proto_file, pool));
   SVN_ERR(svn_io_file_close(proto_file, pool));
 
   /* We don't unlock the prototype revision file immediately to avoid a
@@ -3729,7 +3733,8 @@ commit_body(void *baton, apr_pool_t *poo
   rev_filename = svn_fs_fs__path_rev(cb->fs, new_rev, pool);
   proto_filename = svn_fs_fs__path_txn_proto_rev(cb->fs, txn_id, pool);
   SVN_ERR(svn_fs_fs__move_into_place(proto_filename, rev_filename,
-                                     old_rev_filename, pool));
+                                     old_rev_filename, ffd->flush_to_disk,
+                                     pool));
 
   /* Now that we've moved the prototype revision file out of the way,
      we can unlock it (since further attempts to write to the file
@@ -3741,7 +3746,7 @@ commit_body(void *baton, apr_pool_t *poo
   SVN_ERR_ASSERT(! svn_fs_fs__is_packed_revprop(cb->fs, new_rev));
   revprop_filename = svn_fs_fs__path_revprops(cb->fs, new_rev, pool);
   SVN_ERR(write_final_revprop(revprop_filename, old_rev_filename,
-                              cb->txn, pool));
+                              cb->txn, ffd->flush_to_disk, pool));
 
   /* Update the 'current' file. */
   SVN_ERR(verify_as_revision_before_current_plus_plus(cb->fs, new_rev, pool));

Modified: subversion/trunk/subversion/libsvn_fs_fs/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/util.c?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/util.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/util.c Wed Mar 23 17:39:36 2016
@@ -519,7 +519,8 @@ svn_fs_fs__write_current(svn_fs_t *fs,
 
   name = svn_fs_fs__path_current(fs, pool);
   SVN_ERR(svn_io_write_atomic2(name, buf, strlen(buf),
-                               name /* copy_perms_path */, TRUE, pool));
+                               name /* copy_perms_path */,
+                               ffd->flush_to_disk, pool));
 
   return SVN_NO_ERROR;
 }
@@ -619,6 +620,7 @@ svn_error_t *
 svn_fs_fs__move_into_place(const char *old_filename,
                            const char *new_filename,
                            const char *perms_reference,
+                           svn_boolean_t flush_to_disk,
                            apr_pool_t *pool)
 {
   svn_error_t *err;
@@ -628,7 +630,7 @@ svn_fs_fs__move_into_place(const char *o
   SVN_ERR(svn_io_copy_perms(perms_reference, old_filename, pool));
 
   /* Move the file into place. */
-  err = svn_io_file_rename2(old_filename, new_filename, TRUE, pool);
+  err = svn_io_file_rename2(old_filename, new_filename, flush_to_disk, pool);
   if (err && APR_STATUS_IS_EXDEV(err->apr_err))
     {
       /* Can't rename across devices; fall back to copying. */
@@ -639,25 +641,29 @@ svn_fs_fs__move_into_place(const char *o
          ### The code below is duplicates svn_io_file_rename2(), because
              currently we don't have the svn_io_copy_file2() function with
              a flush_to_disk argument. */
-      SVN_ERR(svn_io_file_open(&file, new_filename, APR_WRITE,
-                               APR_OS_DEFAULT, pool));
-      SVN_ERR(svn_io_file_flush_to_disk(file, pool));
-      SVN_ERR(svn_io_file_close(file, pool));
+      if (flush_to_disk)
+        {
+          SVN_ERR(svn_io_file_open(&file, new_filename, APR_WRITE,
+                                   APR_OS_DEFAULT, pool));
+          SVN_ERR(svn_io_file_flush_to_disk(file, pool));
+          SVN_ERR(svn_io_file_close(file, pool));
+        }
 
 #ifdef SVN_ON_POSIX
-      {
-        /* On POSIX, the file name is stored in the file's directory entry.
-           Hence, we need to fsync() that directory as well.
-           On other operating systems, we'd only be asking for trouble
-           by trying to open and fsync a directory. */
-        const char *dirname;
-
-        dirname = svn_dirent_dirname(new_filename, pool);
-        SVN_ERR(svn_io_file_open(&file, dirname, APR_READ, APR_OS_DEFAULT,
-                                 pool));
-        SVN_ERR(svn_io_file_flush_to_disk(file, pool));
-        SVN_ERR(svn_io_file_close(file, pool));
-      }
+      if (flush_to_disk)
+        {
+          /* On POSIX, the file name is stored in the file's directory entry.
+             Hence, we need to fsync() that directory as well.
+             On other operating systems, we'd only be asking for trouble
+             by trying to open and fsync a directory. */
+          const char *dirname;
+
+          dirname = svn_dirent_dirname(new_filename, pool);
+          SVN_ERR(svn_io_file_open(&file, dirname, APR_READ, APR_OS_DEFAULT,
+                                   pool));
+          SVN_ERR(svn_io_file_flush_to_disk(file, pool));
+          SVN_ERR(svn_io_file_close(file, pool));
+        }
 #endif
     }
   else if (err)

Modified: subversion/trunk/subversion/libsvn_fs_fs/util.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/util.h?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/util.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/util.h Wed Mar 23 17:39:36 2016
@@ -388,11 +388,12 @@ svn_fs_fs__read_number_from_stream(apr_i
    PERMS_REFERENCE.  Temporary allocations are from POOL.
 
    This function almost duplicates svn_io_file_move(), but it tries to
-   guarantee a flush. */
+   guarantee a flush if FLUSH_TO_DISK is non-zero. */
 svn_error_t *
 svn_fs_fs__move_into_place(const char *old_filename,
                            const char *new_filename,
                            const char *perms_reference,
+                           svn_boolean_t flush_to_disk,
                            apr_pool_t *pool);
 
 /* Return TRUE, iff FS uses logical addressing. */

Modified: subversion/trunk/subversion/svnadmin/svnadmin.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/svnadmin.c?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/svnadmin/svnadmin.c (original)
+++ subversion/trunk/subversion/svnadmin/svnadmin.c Wed Mar 23 17:39:36 2016
@@ -72,35 +72,6 @@ warning_func(void *baton,
 }
 
 
-/* Helper to open a repository and set a warning func (so we don't
- * SEGFAULT when libsvn_fs's default handler gets run).  */
-static svn_error_t *
-open_repos(svn_repos_t **repos,
-           const char *path,
-           apr_pool_t *pool)
-{
-  /* Enable the "block-read" feature (where it applies)? */
-  svn_boolean_t use_block_read
-    = svn_cache_config_get()->cache_size > BLOCK_READ_CACHE_THRESHOLD;
-
-  /* construct FS configuration parameters: enable caches for r/o data */
-  apr_hash_t *fs_config = apr_hash_make(pool);
-  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_DELTAS, "1");
-  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS, "1");
-  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_NODEPROPS, "1");
-  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS, "2");
-  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_NS,
-                           svn_uuid_generate(pool));
-  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_BLOCK_READ,
-                           use_block_read ? "1" : "0");
-
-  /* now, open the requested repository */
-  SVN_ERR(svn_repos_open3(repos, path, fs_config, pool, pool));
-  svn_fs_set_warning_func(svn_repos_fs(*repos), warning_func, NULL);
-  return SVN_NO_ERROR;
-}
-
-
 /* Version compatibility check */
 static svn_error_t *
 check_lib_versions(void)
@@ -178,7 +149,8 @@ enum svnadmin__cmdline_options_t
     svnadmin__pre_1_6_compatible,
     svnadmin__compatible_version,
     svnadmin__check_normalization,
-    svnadmin__metadata_only
+    svnadmin__metadata_only,
+    svnadmin__no_flush_to_disk
   };
 
 /* Option codes and descriptions.
@@ -297,6 +269,10 @@ static const apr_getopt_option_t options
         "                             checking against external corruption in\n"
         "                             Subversion 1.9+ format repositories.\n")},
 
+    {"no-flush-to-disk", svnadmin__no_flush_to_disk, 0,
+     N_("disable flushing to disk during the operation\n"
+        "                             (faster, but unsafe on power off)")},
+
     {NULL}
   };
 
@@ -415,7 +391,8 @@ static const svn_opt_subcommand_desc2_t
    {'q', 'r', svnadmin__ignore_uuid, svnadmin__force_uuid,
     svnadmin__ignore_dates,
     svnadmin__use_pre_commit_hook, svnadmin__use_post_commit_hook,
-    svnadmin__parent_dir, svnadmin__bypass_prop_validation, 'M'} },
+    svnadmin__parent_dir, svnadmin__bypass_prop_validation, 'M',
+    svnadmin__no_flush_to_disk} },
 
   {"load-revprops", subcommand_load_revprops, {0}, N_
    ("usage: svnadmin load-revprops REPOS_PATH\n\n"
@@ -561,6 +538,7 @@ struct svnadmin_opt_state
   svn_boolean_t metadata_only;                      /* --metadata-only */
   svn_boolean_t bypass_prop_validation;             /* --bypass-prop-validation */
   svn_boolean_t ignore_dates;                       /* --ignore-dates */
+  svn_boolean_t no_flush_to_disk;                   /* --no-flush-to-disk */
   enum svn_repos_load_uuid uuid_action;             /* --ignore-uuid,
                                                        --force-uuid */
   apr_uint64_t memory_cache_size;                   /* --memory-cache-size M */
@@ -571,6 +549,38 @@ struct svnadmin_opt_state
 };
 
 
+/* Helper to open a repository and set a warning func (so we don't
+ * SEGFAULT when libsvn_fs's default handler gets run).  */
+static svn_error_t *
+open_repos(svn_repos_t **repos,
+           const char *path,
+           struct svnadmin_opt_state *opt_state,
+           apr_pool_t *pool)
+{
+  /* Enable the "block-read" feature (where it applies)? */
+  svn_boolean_t use_block_read
+    = svn_cache_config_get()->cache_size > BLOCK_READ_CACHE_THRESHOLD;
+
+  /* construct FS configuration parameters: enable caches for r/o data */
+  apr_hash_t *fs_config = apr_hash_make(pool);
+  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_DELTAS, "1");
+  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS, "1");
+  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_NODEPROPS, "1");
+  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS, "2");
+  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_NS,
+                           svn_uuid_generate(pool));
+  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_BLOCK_READ,
+                           use_block_read ? "1" : "0");
+  svn_hash_sets(fs_config, SVN_FS_CONFIG_NO_FLUSH_TO_DISK,
+                           opt_state->no_flush_to_disk ? "1" : "0");
+
+  /* now, open the requested repository */
+  SVN_ERR(svn_repos_open3(repos, path, fs_config, pool, pool));
+  svn_fs_set_warning_func(svn_repos_fs(*repos), warning_func, NULL);
+  return SVN_NO_ERROR;
+}
+
+
 /* Set *REVNUM to the revision specified by REVISION (or to
    SVN_INVALID_REVNUM if that has the type 'unspecified'),
    possibly making use of the YOUNGEST revision number in REPOS. */
@@ -677,7 +687,7 @@ subcommand_crashtest(apr_getopt_t *os, v
   svn_repos_t *repos;
 
   (void)svn_error_set_malfunction_handler(crashtest_malfunction_handler);
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   SVN_ERR(svn_cmdline_printf(pool,
                              _("Successfully opened repository '%s'.\n"
                                "Will now crash to simulate a crashing "
@@ -794,7 +804,7 @@ subcommand_deltify(apr_getopt_t *os, voi
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   fs = svn_repos_fs(repos);
   SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
 
@@ -1230,7 +1240,7 @@ subcommand_dump(apr_getopt_t *os, void *
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   SVN_ERR(get_dump_range(&lower, &upper, repos, opt_state, pool));
 
   SVN_ERR(svn_stream_for_stdout(&stdout_stream, pool));
@@ -1261,7 +1271,7 @@ subcommand_dump_revprops(apr_getopt_t *o
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   SVN_ERR(get_dump_range(&lower, &upper, repos, opt_state, pool));
 
   SVN_ERR(svn_stream_for_stdout(&stdout_stream, pool));
@@ -1464,7 +1474,7 @@ subcommand_load(apr_getopt_t *os, void *
      support a limited set of revision kinds: number and unspecified. */
   SVN_ERR(get_load_range(&lower, &upper, opt_state));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
 
   /* Read the stream from STDIN.  Users can redirect a file. */
   SVN_ERR(svn_stream_for_stdin2(&stdin_stream, TRUE, pool));
@@ -1508,7 +1518,7 @@ subcommand_load_revprops(apr_getopt_t *o
      support a limited set of revision kinds: number and unspecified. */
   SVN_ERR(get_load_range(&lower, &upper, opt_state));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
 
   /* Read the stream from STDIN.  Users can redirect a file. */
   SVN_ERR(svn_stream_for_stdin2(&stdin_stream, TRUE, pool));
@@ -1550,7 +1560,7 @@ subcommand_lstxns(apr_getopt_t *os, void
     return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
                              _("Revision range is not allowed"));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   fs = svn_repos_fs(repos);
   SVN_ERR(svn_fs_list_transactions(&txns, fs, pool));
 
@@ -1633,7 +1643,7 @@ subcommand_recover(apr_getopt_t *os, voi
   /* Since db transactions may have been replayed, it's nice to tell
      people what the latest revision is.  It also proves that the
      recovery actually worked. */
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   SVN_ERR(svn_fs_youngest_rev(&youngest_rev, svn_repos_fs(repos), pool));
   SVN_ERR(svn_cmdline_printf(pool, _("The latest repos revision is %ld.\n"),
                              youngest_rev));
@@ -1711,7 +1721,7 @@ subcommand_rmtxns(apr_getopt_t *os, void
 
   SVN_ERR(svn_opt_parse_all_args(&args, os, pool));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   fs = svn_repos_fs(repos);
 
   /* All the rest of the arguments are transaction names. */
@@ -1791,7 +1801,7 @@ set_revprop(const char *prop_name, const
     }
 
   /* Open the filesystem  */
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
 
   if (opt_state->txn_id)
     {
@@ -1867,7 +1877,7 @@ subcommand_setuuid(apr_getopt_t *os, voi
   if (args->nelts == 1)
     uuid = APR_ARRAY_IDX(args, 0, const char *);
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   fs = svn_repos_fs(repos);
   return svn_fs_set_uuid(fs, uuid, pool);
 }
@@ -1915,7 +1925,7 @@ subcommand_pack(apr_getopt_t *os, void *
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
 
   /* Progress feedback goes to STDOUT, unless they asked to suppress it. */
   if (! opt_state->quiet)
@@ -1950,7 +1960,7 @@ subcommand_verify(apr_getopt_t *os, void
                                  "are mutually exclusive"));
     }
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   fs = svn_repos_fs(repos);
   SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
 
@@ -2106,7 +2116,7 @@ subcommand_info(apr_getopt_t *os, void *
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   fs = svn_repos_fs(repos);
   SVN_ERR(svn_cmdline_printf(pool, _("Path: %s\n"),
                              svn_dirent_local_style(svn_repos_path(repos, pool),
@@ -2248,7 +2258,7 @@ subcommand_lock(apr_getopt_t *os, void *
 
   SVN_ERR(target_arg_to_dirent(&comment_file_name, comment_file_name, pool));
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   fs = svn_repos_fs(repos);
 
   /* Create an access context describing the user. */
@@ -2304,7 +2314,7 @@ subcommand_lslocks(apr_getopt_t *os, voi
   if (targets->nelts)
     fs_path = APR_ARRAY_IDX(targets, 0, const char *);
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
 
   /* Fetch all locks on or below the root directory. */
   SVN_ERR(svn_repos_fs_get_locks2(&locks, repos, fs_path, svn_depth_infinity,
@@ -2362,7 +2372,7 @@ subcommand_rmlocks(apr_getopt_t *os, voi
   const char *username;
   apr_pool_t *subpool = svn_pool_create(pool);
 
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   fs = svn_repos_fs(repos);
 
   /* svn_fs_unlock() demands that some username be associated with the
@@ -2453,7 +2463,7 @@ subcommand_unlock(apr_getopt_t *os, void
 
   /* Open the repos/FS, and associate an access context containing
      USERNAME. */
-  SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
+  SVN_ERR(open_repos(&repos, opt_state->repository_path, opt_state, pool));
   fs = svn_repos_fs(repos);
   SVN_ERR(svn_fs_create_access(&access, username, pool));
   SVN_ERR(svn_fs_set_access(fs, access));
@@ -2798,6 +2808,9 @@ sub_main(int *exit_code, int argc, const
       case svnadmin__wait:
         opt_state.wait = TRUE;
         break;
+      case svnadmin__no_flush_to_disk:
+        opt_state.no_flush_to_disk = TRUE;
+        break;
       default:
         {
           SVN_ERR(subcommand_help(NULL, NULL, pool));

Modified: subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py Wed Mar 23 17:39:36 2016
@@ -3367,6 +3367,23 @@ def dump_no_op_prop_change(sbox):
   svntest.actions.run_and_verify_svn(expected, [], 'log',  '-v',
                                      sbox2.repo_url + '/bar')
 
+def load_no_flush_to_disk(sbox):
+  "svnadmin load --no-flush-to-disk"
+
+  sbox.build(empty=True)
+
+  # Can't test the "not flushing to disk part", but loading the
+  # dump should work.
+  dump = clean_dumpfile()
+  expected = [
+    svntest.wc.State('', {
+      'A' : svntest.wc.StateItem(contents="text\n",
+                                 props={'svn:keywords': 'Id'})
+      })
+    ]
+  load_and_verify_dumpstream(sbox, [], [], expected, True, dump,
+                             '--no-flush-to-disk', '--ignore-uuid')
+
 ########################################################################
 # Run the tests
 
@@ -3428,7 +3445,8 @@ test_list = [ None,
               load_revprops,
               dump_revprops,
               dump_no_op_change,
-              dump_no_op_prop_change
+              dump_no_op_prop_change,
+              load_no_flush_to_disk
              ]
 
 if __name__ == '__main__':

Modified: subversion/trunk/tools/client-side/bash_completion
URL: http://svn.apache.org/viewvc/subversion/trunk/tools/client-side/bash_completion?rev=1736357&r1=1736356&r2=1736357&view=diff
==============================================================================
--- subversion/trunk/tools/client-side/bash_completion (original)
+++ subversion/trunk/tools/client-side/bash_completion Wed Mar 23 17:39:36 2016
@@ -1150,7 +1150,8 @@ _svnadmin ()
 	load)
 		cmdOpts="--ignore-uuid --force-uuid --parent-dir -q --quiet \
 		         --use-pre-commit-hook --use-post-commit-hook \
-		         --bypass-prop-validation -M --memory-cache-size"
+		         --bypass-prop-validation -M --memory-cache-size \
+		         --no-flush-to-disk"
 		;;
 	lstxns)
         	cmdOpts="-r --revision"



Re: svn commit: r1736357 - in /subversion/trunk: subversion/include/ subversion/libsvn_fs_fs/ subversion/svnadmin/ subversion/tests/cmdline/ tools/client-side/

Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Ivan Zhakov <iv...@apache.org> writes:

>>    fs_fs_data_t *ffd = apr_pcalloc(fs->pool, sizeof(*ffd));
>>    ffd->use_log_addressing = FALSE;
>>    ffd->revprop_prefix = 0;
>> +  ffd->flush_to_disk = FALSE;
>
> I think would be more safe to initialize FLUSH_TO_DISK to TRUE to
> match old behavior and prevent possible mistakes in future when some
> other codepath will initialize_fs_struct() without calling
> read_global_config().

Agreed, will do.

>>    SVN_ERR(svn_fs_fs__move_into_place(tmp_path, final_path, perms_reference,
>> -                                     pool));
>> +                                     TRUE, pool));
>
> Why ffd->flush_to_disk() is not used here? As far I remember
> revprops-only loading from dump is one of the use case and we already
> have optimized codepath for them.

My intent was not to change more calling sites than necessary.  However, in
this particular case that's probably an oversight, at least from the point of
anyone using SVN_FS_CONFIG_NO_FLUSH_TO_DISK.

I'll update this call, and will also add the --no-flush-to-disk option to
the `svnadmin load-revprops' subcommand, since it makes sense there
as well.


Thanks,
Evgeny Kotkov

Re: svn commit: r1736357 - in /subversion/trunk: subversion/include/ subversion/libsvn_fs_fs/ subversion/svnadmin/ subversion/tests/cmdline/ tools/client-side/

Posted by Ivan Zhakov <iv...@apache.org>.
 On 23 March 2016 at 20:39,  <ko...@apache.org> wrote:
> Author: kotkov
> Date: Wed Mar 23 17:39:36 2016
> New Revision: 1736357
>
> URL: http://svn.apache.org/viewvc?rev=1736357&view=rev
> Log:
> Introduce `--no-flush-to-disk' option for `svnadmin load'.
>
> The option can be used to to dramatically speed up the load process when
> there's no need to ensure that the resulting data survives a system crash
> or power loss — e.g., when loading a dump into a fresh new repository.
>
> This is one of the ideas in http://svn.haxx.se/dev/archive-2015-09/0187.shtml
> (Subject: "Whiteboard -- topics list on the white board").
>
Nice option, see my review inline.

[...]

> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.c?rev=1736357&r1=1736356&r2=1736357&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs.c Wed Mar 23 17:39:36 2016
> @@ -292,6 +292,7 @@ initialize_fs_struct(svn_fs_t *fs)
>    fs_fs_data_t *ffd = apr_pcalloc(fs->pool, sizeof(*ffd));
>    ffd->use_log_addressing = FALSE;
>    ffd->revprop_prefix = 0;
> +  ffd->flush_to_disk = FALSE;
I think would be more safe to initialize FLUSH_TO_DISK to TRUE to
match old behavior and prevent possible mistakes in future when some
other codepath will initialize_fs_struct() without calling
read_global_config().

[...]

> Modified: subversion/trunk/subversion/libsvn_fs_fs/revprops.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/revprops.c?rev=1736357&r1=1736356&r2=1736357&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/revprops.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/revprops.c Wed Mar 23 17:39:36 2016
> @@ -776,7 +776,7 @@ switch_to_new_revprop(svn_fs_t *fs,
>                        apr_pool_t *pool)
>  {
>    SVN_ERR(svn_fs_fs__move_into_place(tmp_path, final_path, perms_reference,
> -                                     pool));
> +                                     TRUE, pool));
Why ffd->flush_to_disk() is not used here? As far I remember
revprops-only loading from dump is one of the use case and we already
have optimized codepath for them.


-- 
Ivan Zhakov