You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2012/10/31 13:40:53 UTC
RE: svn commit: r1404112 - in /subversion/trunk/subversion: include/private/svn_named_atomic.h libsvn_fs_fs/fs_fs.c libsvn_subr/named_atomic.c tests/cmdline/svnadmin_tests.py tests/libsvn_subr/named_atomic-test.c
> -----Original Message-----
> From: stefan2@apache.org [mailto:stefan2@apache.org]
> Sent: woensdag 31 oktober 2012 13:27
> To: commits@subversion.apache.org
> Subject: svn commit: r1404112 - in /subversion/trunk/subversion:
> include/private/svn_named_atomic.h libsvn_fs_fs/fs_fs.c
> libsvn_subr/named_atomic.c tests/cmdline/svnadmin_tests.py
> tests/libsvn_subr/named_atomic-test.c
>
> Author: stefan2
> Date: Wed Oct 31 12:27:29 2012
> New Revision: 1404112
>
> URL: http://svn.apache.org/viewvc?rev=1404112&view=rev
> Log:
> Change the way we implement shared memory setup for our named
> atomics.
> Instead of using APR's SHM API, we create a persistent file and simply
> mmap it.
>
> The only downside to this is that the SHM file does not get cleaned up
> automatically anymore. Therefore, we need to update tests and hotcopy
> code.
>
> The underlying issue has been analyzed in this thread:
> http://svn.haxx.se/dev/archive-2012-10/0423.shtml
>
> * subversion/include/private/svn_named_atomic.h
> (svn_atomic_namespace__cleanup): declare new API
>
> * subversion/libsvn_subr/named_atomic.c
> (): update global docstring
> (svn_atomic_namespace__create): create a persistent file; mmap it
> (svn_atomic_namespace__cleanup): implement new API
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (cleanup_revprop_namespace): new utility function
> (hotcopy_body): fix comment; remove temp atomics files
>
> * subversion/tests/cmdline/svnadmin_tests.py
> (check_hotcopy_fsfs): don't compare temp files related to atomics
>
> * subversion/tests/libsvn_subr/named_atomic-test.c
> (cleanup_test_shm): new cleanup function
> test_basics,
> test_bignums,
> test_multiple_atomics,
> test_namespaces,
> test_multithreaded,
> test_multiprocess): call the new function
>
> Modified:
> subversion/trunk/subversion/include/private/svn_named_atomic.h
> subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> subversion/trunk/subversion/libsvn_subr/named_atomic.c
> subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
> subversion/trunk/subversion/tests/libsvn_subr/named_atomic-test.c
>
> Modified:
> subversion/trunk/subversion/include/private/svn_named_atomic.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private
> /svn_named_atomic.h?rev=1404112&r1=1404111&r2=1404112&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/include/private/svn_named_atomic.h
> (original)
> +++ subversion/trunk/subversion/include/private/svn_named_atomic.h
> Wed Oct 31 12:27:29 2012
> @@ -83,6 +83,17 @@ svn_atomic_namespace__create(svn_atomic_
> const char *name,
> apr_pool_t *result_pool);
>
> +/** Removes persistent data structures (files in particular) that got
> + * created for the namespace given by @a name. Use @a pool for
> temporary
> + * allocations.
> + *
> + * @note You must not call this while the respective namespace is still
> + * in use. Calling this multiple times for the same namespace is safe.
> + */
> +svn_error_t *
> +svn_atomic_namespace__cleanup(const char *name,
> + apr_pool_t *pool);
> +
> /** Find the atomic with the specified @a name in namespace @a ns and
> * return it in @a *atomic. If no object with that name can be found, the
> * behavior depends on @a auto_create. If it is @c FALSE, @a *atomic will
>
> 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=1404112&r1=1404111&r2=1404112&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Oct 31 12:27:29
> 2012
> @@ -3160,6 +3160,16 @@ ensure_revprop_namespace(svn_fs_t *fs)
> : SVN_NO_ERROR;
> }
>
> +/* Make sure the revprop_namespace member in FS is set. */
> +static svn_error_t *
> +cleanup_revprop_namespace(svn_fs_t *fs)
> +{
> + const char* name = svn_dirent_join(fs->path,
> + ATOMIC_REVPROP_NAMESPACE,
> + fs->pool);
> + return svn_error_trace(svn_atomic_namespace__cleanup(name, fs-
> >pool));
> +}
> +
> /* Make sure the revprop_generation member in FS is set and, if necessary,
> * initialized with the latest value stored on disk.
> */
> @@ -11035,14 +11045,16 @@ hotcopy_body(void *baton, apr_pool_t *po
> PATH_TXN_CURRENT, pool));
>
> /* If a revprop generation file exists in the source filesystem,
> - * force a fresh revprop caching namespace for the destination by
> - * setting the generation to zero. We have no idea if the revprops
> - * we copied above really belong to the currently cached generation. */
> + * reset it to zero (since this is on a different path, it will not
> + * overlap with data already in cache). Also, clean up stale files
> + * used for the named atomics implementation. */
> SVN_ERR(svn_io_check_path(path_revprop_generation(src_fs, pool),
> &kind, pool));
> if (kind == svn_node_file)
> SVN_ERR(write_revprop_generation_file(dst_fs, 0, pool));
>
> + SVN_ERR(cleanup_revprop_namespace(dst_fs));
> +
> /* Hotcopied FS is complete. Stamp it with a format file. */
> SVN_ERR(write_format(svn_dirent_join(dst_fs->path, PATH_FORMAT,
> pool),
> dst_ffd->format, max_files_per_dir, TRUE, pool));
>
> Modified: subversion/trunk/subversion/libsvn_subr/named_atomic.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/na
> med_atomic.c?rev=1404112&r1=1404111&r2=1404112&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_subr/named_atomic.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/named_atomic.c Wed Oct 31
> 12:27:29 2012
> @@ -24,7 +24,7 @@
> #include "private/svn_named_atomic.h"
>
> #include <apr_global_mutex.h>
> -#include <apr_shm.h>
> +#include <apr_mmap.h>
>
> #include "svn_private_config.h"
> #include "private/svn_atomic.h"
> @@ -35,13 +35,16 @@
>
> /* Implementation aspects.
> *
> - * We use a single shared memory block that will be created by the first
> - * user and merely mapped by all subsequent ones. The memory block
> contains
> - * an short header followed by a fixed-capacity array of named atomics. The
> - * number of entries currently in use is stored in the header part.
> + * We use a single shared memory block (memory mapped file) that will be
> + * created by the first user and merely mapped by all subsequent ones.
> + * The memory block contains an short header followed by a fixed-capacity
> + * array of named atomics. The number of entries currently in use is stored
> + * in the header part.
> *
> - * Finding / creating the SHM object as well as adding new array entries
> - * is being guarded by an APR global mutex.
> + * Finding / creating the MMAP object as well as adding new array entries
> + * is being guarded by an APR global mutex. Since releasing the MMAP
> + * structure and closing the underlying does not affect other users of the
> + * same, cleanup will not be synchronized.
> *
> * The array is append-only. Once a process mapped the block into its
> * address space, it may freely access any of the used entries. However,
> @@ -382,9 +385,13 @@ svn_atomic_namespace__create(svn_atomic_
> apr_pool_t *result_pool)
> {
> apr_status_t apr_err;
> + svn_error_t *err;
> + apr_file_t *file;
> + apr_mmap_t *mmap;
> const char *shm_name, *lock_name;
> - apr_shm_t *shared_mem;
> - int i;
> + svn_node_kind_t kind;
> +
> + apr_pool_t *sub_pool = svn_pool_create(result_pool);
Why do you create a subpool if you can just make the caller pass a scratch_pool?
(Usualle we use 'subpool' and 'iterpool' for the variable name, not '(sub|iter)_pool')
Bert