You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/03/03 12:31:17 UTC
svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs:
caching.c fs.h fs_fs.c
Author: stefan2
Date: Sat Mar 3 11:31:17 2012
New Revision: 1296604
URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
Log:
Certain operations, e.g. svn ls, will contain timestamp and author
information from many different revisions. A list of all projects
in the root of the wordpress repository will open, read and close
>75.000 revision property files (3 reads for each list entry)
This commit implements revprop caching. It will be activated as
part of the full-text caching option.
Since revprops may be written by other threads or processes, we
need to track the revprop changes. A new special file contains a
counter that will be increased each time revision properties get
rewritten.
This counter is internally called "revprop generation" and will be
read upon the first revprop access for given fs_t. Later changes
may remain invisible for that fs_t. This behavior is in line with
our revprop handling in other parts of FS_FS. If a revprop gets
rewritten, the fs_t doing the write will use the new generation
from that point on and will thus see all caches up to and including
its own.
Since the revprop generation becomes part of the cache key, each
fs_t will only see revprops from its generation. It may also
create new cache entries tagged with that generation, i.e. those
would appear to be outdated for newer fs_t. But that will simply
cause a benign false negative upon lookup. No fs_t will see
data that got replaced before that fs_t was created.
* subversion/libsvn_fs_fs/fs.h
(PATH_REVPROP_GEN): declare name of new special file
(fs_fs_data_t): add member to hold that file's content;
add new cache for revprops
* subversion/libsvn_fs_fs/caching.c
(svn_fs_fs__initialize_caches): initialize revprop cache
* subversion/libsvn_fs_fs/fs_fs.c
(path_revprop_generation): path constructor for new special file
(check_file_buffer_numeric): new, more generic utility function
(check_format_file_buffer_numeric): call the new utility
(read_revprop_generation, check_revprop_generation,
increment_revprop_generation): new functions to read & write
the revprop generation from / to our new special file
(set_revision_proplist): if we change existing revprops, increment
the revprop generation
(revision_proplist): use and populate the revprop cache
(svn_fs_fs__create): initialize our new special file
(hotcopy_create_empty_dest): hotcopy the new special file
Modified:
subversion/trunk/subversion/libsvn_fs_fs/caching.c
subversion/trunk/subversion/libsvn_fs_fs/fs.h
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/caching.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/caching.c?rev=1296604&r1=1296603&r2=1296604&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/caching.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/caching.c Sat Mar 3 11:31:17 2012
@@ -339,6 +339,27 @@ svn_fs_fs__initialize_caches(svn_fs_t *f
SVN_ERR(init_callbacks(ffd->fulltext_cache, fs, no_handler, pool));
+ /* initialize revprop cache, if full-text caching has been enabled */
+ if (cache_fulltexts)
+ {
+ SVN_ERR(create_cache(&(ffd->revprop_cache),
+ NULL,
+ membuffer,
+ 0, 0, /* Do not use inprocess cache */
+ svn_fs_fs__serialize_properties,
+ svn_fs_fs__deserialize_properties,
+ APR_HASH_KEY_STRING,
+ apr_pstrcat(pool, prefix, "REVPROP",
+ (char *)NULL),
+ fs->pool));
+ }
+ else
+ {
+ ffd->revprop_cache = NULL;
+ }
+
+ SVN_ERR(init_callbacks(ffd->revprop_cache, fs, no_handler, pool));
+
/* initialize txdelta window cache, if that has been enabled */
if (cache_txdeltas)
{
Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.h?rev=1296604&r1=1296603&r2=1296604&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs.h Sat Mar 3 11:31:17 2012
@@ -60,6 +60,8 @@ extern "C" {
#define PATH_LOCKS_DIR "locks" /* Directory of locks */
#define PATH_MIN_UNPACKED_REV "min-unpacked-rev" /* Oldest revision which
has not been packed. */
+#define PATH_REVPROP_GEN "revprop-gen" /* File containing the
+ revprop change counter */
/* If you change this, look at tests/svn_test_fs.c(maybe_install_fsfs_conf) */
#define PATH_CONFIG "fsfs.conf" /* Configuration */
@@ -198,7 +200,8 @@ typedef struct fs_fs_shared_data_t
apr_pool_t *common_pool;
} fs_fs_shared_data_t;
-/* Private (non-shared) FSFS-specific data for each svn_fs_t object. */
+/* Private (non-shared) FSFS-specific data for each svn_fs_t object.
+ Any caches in here may be NULL. */
typedef struct fs_fs_data_t
{
/* The format number of this FS. */
@@ -237,6 +240,15 @@ typedef struct fs_fs_data_t
rep key (revision/offset) to svn_string_t. */
svn_cache__t *fulltext_cache;
+ /* Revprop "generation" that is valid for this svn_fs_t.
+ It is the revprop change counter read once from "revprop-gen".
+ Will be read upon first access. 0 means that the value has not
+ been read from disk, yet. */
+ apr_int64_t revprop_generation;
+
+ /* Revision property cache. Maps from (rev,generation) to apr_hash_t. */
+ svn_cache__t *revprop_cache;
+
/* Pack manifest cache; a cache mapping (svn_revnum_t) shard number to
a manifest; and a manifest is a mapping from (svn_revnum_t) revision
number offset within a shard to (apr_off_t) byte-offset in the
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=1296604&r1=1296603&r2=1296604&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sat Mar 3 11:31:17 2012
@@ -225,6 +225,12 @@ path_lock(svn_fs_t *fs, apr_pool_t *pool
}
static const char *
+path_revprop_generation(svn_fs_t *fs, apr_pool_t *pool)
+{
+ return svn_dirent_join(fs->path, PATH_REVPROP_GEN, pool);
+}
+
+static const char *
path_rev_packed(svn_fs_t *fs, svn_revnum_t rev, const char *kind,
apr_pool_t *pool)
{
@@ -870,25 +876,39 @@ get_file_offset(apr_off_t *offset_p, apr
}
-/* Check that BUF, a nul-terminated buffer of text from format file PATH,
+/* Check that BUF, a nul-terminated buffer of text from file PATH,
contains only digits at OFFSET and beyond, raising an error if not.
+ TITLE contains a user-visible description of the file, usually the
+ short file name.
Uses POOL for temporary allocation. */
static svn_error_t *
-check_format_file_buffer_numeric(const char *buf, apr_off_t offset,
- const char *path, apr_pool_t *pool)
+check_file_buffer_numeric(const char *buf, apr_off_t offset,
+ const char *path, const char *title,
+ apr_pool_t *pool)
{
const char *p;
for (p = buf + offset; *p; p++)
if (!svn_ctype_isdigit(*p))
return svn_error_createf(SVN_ERR_BAD_VERSION_FILE_FORMAT, NULL,
- _("Format file '%s' contains unexpected non-digit '%c' within '%s'"),
- svn_dirent_local_style(path, pool), *p, buf);
+ _("%s file '%s' contains unexpected non-digit '%c' within '%s'"),
+ title, svn_dirent_local_style(path, pool), *p, buf);
return SVN_NO_ERROR;
}
+/* Check that BUF, a nul-terminated buffer of text from format file PATH,
+ contains only digits at OFFSET and beyond, raising an error if not.
+
+ Uses POOL for temporary allocation. */
+static svn_error_t *
+check_format_file_buffer_numeric(const char *buf, apr_off_t offset,
+ const char *path, apr_pool_t *pool)
+{
+ return check_file_buffer_numeric(buf, offset, path, "Format", pool);
+}
+
/* Read the format number and maximum number of files per directory
from PATH and return them in *PFORMAT and *MAX_FILES_PER_DIR
respectively.
@@ -2687,6 +2707,104 @@ svn_fs_fs__rev_get_root(svn_fs_id_t **ro
return SVN_NO_ERROR;
}
+/* Reads the revprop_gen file and writes the content into the
+ REVPROP_GENERATION member of FS. Use pool for allocations. */
+static svn_error_t *
+read_revprop_generation(svn_fs_t *fs,
+ apr_pool_t *pool)
+{
+ fs_fs_data_t *ffd = fs->fsap_data;
+
+ const char *path = path_revprop_generation(fs, pool);
+ svn_error_t *err;
+ char buf[80];
+ int i;
+
+ /* Read the raw data from the file, if it exists. If it does
+ not, set the generation to "1" and return.
+ We don't want to have this function fail. So, patiently
+ retry a couple of times the case the OS denied us access. */
+ apr_pool_t *iterpool = svn_pool_create(pool);
+ for (i = 0; i < RECOVERABLE_RETRY_COUNT; ++i)
+ {
+ apr_file_t *file;
+ apr_size_t len = sizeof(buf);
+
+ svn_pool_clear(iterpool);
+
+ err = svn_io_file_open(&file, path, APR_READ | APR_BUFFERED,
+ APR_OS_DEFAULT, iterpool);
+ if (err && APR_STATUS_IS_ENOENT(err->apr_err))
+ {
+ /* No-one changed a revprop -> we are still at gen 1. */
+ ffd->revprop_generation = 1;
+ svn_error_clear(err);
+ return SVN_NO_ERROR;
+ }
+ svn_error_clear(err);
+
+ RETRY_RECOVERABLE(err, file,
+ svn_io_read_length_line(file,
+ buf,
+ &len,
+ iterpool));
+ IGNORE_RECOVERABLE(err, svn_io_file_close(file,
+ iterpool));
+
+ break;
+ }
+ SVN_ERR(err);
+
+ svn_pool_destroy(iterpool);
+
+ /* Check that the first line contains only digits. */
+ SVN_ERR(check_file_buffer_numeric(buf, 0, path,
+ "Revprop generations", pool));
+ SVN_ERR(svn_cstring_atoi64(&ffd->revprop_generation, buf));
+
+ /* Graceful behavior in case someone put a "0" in the file. */
+ if (ffd->revprop_generation <= 0)
+ ffd->revprop_generation = 1;
+
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+check_revprop_generation(svn_fs_t *fs,
+ apr_pool_t *pool)
+{
+ fs_fs_data_t *ffd = fs->fsap_data;
+
+ return ffd->revprop_generation == 0
+ ? read_revprop_generation(fs, pool)
+ : SVN_NO_ERROR;
+}
+
+static svn_error_t *
+increment_revprop_generation(svn_fs_t *fs,
+ apr_pool_t *pool)
+{
+ fs_fs_data_t *ffd = fs->fsap_data;
+
+ SVN_ERR(read_revprop_generation(fs, pool));
+
+ const char *path = path_revprop_generation(fs, pool);
+ const char *tmp_filename;
+ svn_string_t *generation;
+
+ /* Increment the key and add a trailing \n to the string so the
+ txn-current file has a newline in it. */
+ ++ffd->revprop_generation;
+ generation = svn_string_createf(pool, "%ld\n", ffd->revprop_generation);
+
+ SVN_ERR(svn_io_write_unique(&tmp_filename,
+ svn_dirent_dirname(path, pool),
+ generation->data, generation->len,
+ svn_io_file_del_none, pool));
+ return move_into_place(tmp_filename, path,
+ svn_fs_fs__path_current(fs, pool), pool);
+}
+
/* Set the revision property list of revision REV in filesystem FS to
PROPLIST. Use POOL for temporary allocations. */
static svn_error_t *
@@ -2703,6 +2821,10 @@ set_revision_proplist(svn_fs_t *fs,
const char *tmp_path;
const char *perms_reference;
svn_stream_t *stream;
+ svn_node_kind_t kind;
+
+ /* test whether revprops already exist for this revision */
+ SVN_ERR(svn_io_check_path(final_path, &kind, pool));
/* ### do we have a directory sitting around already? we really shouldn't
### have to get the dirname here. */
@@ -2719,6 +2841,12 @@ set_revision_proplist(svn_fs_t *fs,
SVN_ERR(svn_fs_fs__path_rev_absolute(&perms_reference, fs, rev, pool));
SVN_ERR(move_into_place(tmp_path, final_path, perms_reference, pool));
+ /* Invalidate all cached revprops for this FS and for all other
+ users that haven't read any revprops, YET. Since writing revprops
+ implies a write lock, there can be no races. */
+ if (kind != svn_node_none)
+ SVN_ERR(increment_revprop_generation(fs, pool));
+
return SVN_NO_ERROR;
}
@@ -2732,8 +2860,22 @@ revision_proplist(apr_hash_t **proplist_
apr_pool_t *pool)
{
apr_hash_t *proplist;
+ fs_fs_data_t *ffd = fs->fsap_data;
+ const char *key;
SVN_ERR(ensure_revision_exists(fs, rev, pool));
+ SVN_ERR(check_revprop_generation(fs, pool));
+
+ /* Try cache lookup first. */
+ key = svn_fs_fs__combine_two_numbers(rev, ffd->revprop_generation, pool);
+ if (ffd->revprop_cache)
+ {
+ svn_boolean_t is_cached;
+ SVN_ERR(svn_cache__get((void **) proplist_p, &is_cached,
+ ffd->revprop_cache, key, pool));
+ if (is_cached)
+ return SVN_NO_ERROR;
+ }
/* if (1); null condition for easier merging to revprop-packing */
{
@@ -2789,6 +2931,10 @@ revision_proplist(apr_hash_t **proplist_
svn_pool_destroy(iterpool);
}
+ /* Cache the result, if caching has been activated. */
+ if (ffd->revprop_cache)
+ SVN_ERR(svn_cache__set(ffd->revprop_cache, key, proplist, pool));
+
*proplist_p = proplist;
return SVN_NO_ERROR;
@@ -6762,6 +6908,9 @@ svn_fs_fs__create(svn_fs_t *fs,
"", pool));
}
+ /* Create the revprop generation tracking file. */
+ SVN_ERR(increment_revprop_generation(fs, pool));
+
/* This filesystem is ready. Stamp it with a format number. */
SVN_ERR(write_format(path_format(fs, pool),
ffd->format, ffd->max_files_per_dir, FALSE, pool));
@@ -8812,6 +8961,7 @@ hotcopy_create_empty_dest(svn_fs_t *src_
{
fs_fs_data_t *src_ffd = src_fs->fsap_data;
fs_fs_data_t *dst_ffd = dst_fs->fsap_data;
+ svn_node_kind_t kind;
dst_fs->path = apr_pstrdup(pool, dst_path);
@@ -8875,6 +9025,14 @@ hotcopy_create_empty_dest(svn_fs_t *src_
"", pool));
}
+ /* Copy the revprop generation file if it exists in SRC_FS. */
+ SVN_ERR(svn_io_check_path(path_revprop_generation(src_fs, pool),
+ &kind, pool));
+ if (kind == svn_node_file)
+ SVN_ERR(svn_io_copy_file(path_revprop_generation(src_fs, pool),
+ path_revprop_generation(dst_fs, pool),
+ TRUE, pool));
+
dst_ffd->youngest_rev_cache = 0;
return SVN_NO_ERROR;
}
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Fuhrmann <eq...@web.de> writes:
> On 06.03.2012 10:43, Philip Martin wrote:
>> Perhaps the read routines could be changed to always read the
>> revprop-gen file and refresh the cache if required. Obviously this
>> would involve more IO than the current caching strategy, but it would
>> still be more efficient than the the previous non-cached approach.
>>
> Actually, this is a good idea -- maybe with
> some lower rating on "style" (adding a one-
> liner to most of the FS_FS API functions).
>
> And we don't have to actually re-read the
> revprop-gen file but only need to invalidate
> the cached revprop-gen value. Only the
> next time we read some revprop, will the
> generation info be read from file again.
>
> I'll implement that in the next couple of days.
I don't quite follow. I was thinking that the function that retrieves
the value from the cache would check the revprop-gen file, so only that
one function changes. It does reread the revprop-gen file, and if
necessary rereads the revprop file. I suppose the write function might
also change so that if the cache is known to be out-of-date then reading
revprop-gen can be skipped.
You seem to be describing something else?
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs:
caching.c fs.h fs_fs.c
Posted by Stefan Fuhrmann <eq...@web.de>.
On 06.03.2012 10:43, Philip Martin wrote:
> Daniel Shahaf<da...@elego.de> writes:
>
>> I believe you cannot assume svn_fs_t's will be short lived, and
>> _certainly_ cannot assume that the access pattern to an svn_fs_t
>> will be in any way related to random clients having random RA
>> sessions; for all the FS API knows, svnserve is a daemon that calls
>> svn_fs_open() once per reboot.
>>
>> And yes, people do this.
> Perhaps the read routines could be changed to always read the
> revprop-gen file and refresh the cache if required. Obviously this
> would involve more IO than the current caching strategy, but it would
> still be more efficient than the the previous non-cached approach.
>
Actually, this is a good idea -- maybe with
some lower rating on "style" (adding a one-
liner to most of the FS_FS API functions).
And we don't have to actually re-read the
revprop-gen file but only need to invalidate
the cached revprop-gen value. Only the
next time we read some revprop, will the
generation info be read from file again.
I'll implement that in the next couple of days.
-- Stefan^2.
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> I believe you cannot assume svn_fs_t's will be short lived, and
> _certainly_ cannot assume that the access pattern to an svn_fs_t
> will be in any way related to random clients having random RA
> sessions; for all the FS API knows, svnserve is a daemon that calls
> svn_fs_open() once per reboot.
>
> And yes, people do this.
Perhaps the read routines could be changed to always read the
revprop-gen file and refresh the cache if required. Obviously this
would involve more IO than the current caching strategy, but it would
still be more efficient than the the previous non-cached approach.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Stefan Fuhrmann wrote on Mon, Mar 05, 2012 at 22:57:30 +0100:
> On 05.03.2012 15:20, Daniel Shahaf wrote:
> >Philip Martin wrote on Mon, Mar 05, 2012 at 13:58:18 +0000:
> >>Daniel Shahaf<da...@elego.de> writes:
> >>
> >>>Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
> >>>>Daniel Shahaf<da...@elego.de> writes:
> >>>>
> >>>>>You're right, I misread the code: I mistakenly thought line 2867 will
> >>>>>always re-read the revprop-gen file from disk. How about:
> >>>>>
> >>>>>Index: subversion/libsvn_fs_fs/fs_fs.c
> >>>>>===================================================================
> >>>>>--- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
> >>>>>+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> >>>>>@@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
> >>>>> fs_fs_data_t *ffd = fs->fsap_data;
> >>>>> if (ffd->format>= SVN_FS_FS__MIN_PACKED_FORMAT)
> >>>>> SVN_ERR(update_min_unpacked_rev(fs, pool));
> >>>>> SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
> >>>>> pool));
> >>>>>+ SVN_ERR(read_revprop_generation(fs, pool));
> >>>>> err = body(baton, subpool);
> >>>>> }
> >>>>>
> >>>>That looks like it works. But it only works if all writers update the
> >>>>generation file so this whole caching scheme requires an FSFS format
> >>>>bump to exclude 1.7 and earlier.
> >>>+1
> >>>
> >>>(It was pointed on IRC, too.)
> >>If we were to clear the cache after taking the write lock, either
> >>unconditionally or if the revprop generation has changed, then we would
> >>become compatible with pre-1.8 style writers.
> >>
> >>Of course this does highlight a change in svn_fs_t behaviour. A long
> >>lived svn_fs_t may never see revprop updates as the cache never expires.
> >>The cached values might get pushed out if the process (thead?) reads
> >>other revisions, to get reasonably up-to-date values the caller must not
> >>hold svn_fs_t objects too long.
> >Which is a regression --- even a process that does proplist() in a loop
> >should eventually see changes.
>
> Hm. I would expect the live of a fs_t to be limited
> by the life of the RA session. At least for our CL
> client, this would not be a problem.
>
> For TSVN dialogs like the repository browser that
> keep client contexts open for a long time, it all
> hinges on how long the RA session is kept open
> by the SVN libs and whether the client has any
> control over it.
>
> Can anyone comment on that?
>
I believe you cannot assume svn_fs_t's will be short lived, and
_certainly_ cannot assume that the access pattern to an svn_fs_t
will be in any way related to random clients having random RA
sessions; for all the FS API knows, svnserve is a daemon that calls
svn_fs_open() once per reboot.
And yes, people do this.
> -- Stefan^2.
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs:
caching.c fs.h fs_fs.c
Posted by Blair Zajac <bl...@orcaware.com>.
On 03/05/2012 01:57 PM, Stefan Fuhrmann wrote:
> On 05.03.2012 15:20, Daniel Shahaf wrote:
>> Philip Martin wrote on Mon, Mar 05, 2012 at 13:58:18 +0000:
>>> Daniel Shahaf<da...@elego.de> writes:
>>>
>>>> Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
>>>>> Daniel Shahaf<da...@elego.de> writes:
>>>>>
>>>>>> You're right, I misread the code: I mistakenly thought line 2867 will
>>>>>> always re-read the revprop-gen file from disk. How about:
>>>>>>
>>>>>> Index: subversion/libsvn_fs_fs/fs_fs.c
>>>>>> ===================================================================
>>>>>> --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
>>>>>> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
>>>>>> @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
>>>>>> fs_fs_data_t *ffd = fs->fsap_data;
>>>>>> if (ffd->format>= SVN_FS_FS__MIN_PACKED_FORMAT)
>>>>>> SVN_ERR(update_min_unpacked_rev(fs, pool));
>>>>>> SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
>>>>>> pool));
>>>>>> + SVN_ERR(read_revprop_generation(fs, pool));
>>>>>> err = body(baton, subpool);
>>>>>> }
>>>>>>
>>>>> That looks like it works. But it only works if all writers update the
>>>>> generation file so this whole caching scheme requires an FSFS format
>>>>> bump to exclude 1.7 and earlier.
>>>> +1
>>>>
>>>> (It was pointed on IRC, too.)
>>> If we were to clear the cache after taking the write lock, either
>>> unconditionally or if the revprop generation has changed, then we would
>>> become compatible with pre-1.8 style writers.
>>>
>>> Of course this does highlight a change in svn_fs_t behaviour. A long
>>> lived svn_fs_t may never see revprop updates as the cache never expires.
>>> The cached values might get pushed out if the process (thead?) reads
>>> other revisions, to get reasonably up-to-date values the caller must not
>>> hold svn_fs_t objects too long.
>> Which is a regression --- even a process that does proplist() in a loop
>> should eventually see changes.
>
> Hm. I would expect the live of a fs_t to be limited
> by the life of the RA session. At least for our CL
> client, this would not be a problem.
>
> For TSVN dialogs like the repository browser that
> keep client contexts open for a long time, it all
> hinges on how long the RA session is kept open
> by the SVN libs and whether the client has any
> control over it.
>
> Can anyone comment on that?
We (Sony Imageworks) have a custom built Subversion server that does
2,000 RPCs/sec, so we LRU cache svn_fs_t, svn_repos_t, and svn_fs_root_t
to avoid filesystem IO.
Blair
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs:
caching.c fs.h fs_fs.c
Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
On 05.03.2012 15:20, Daniel Shahaf wrote:
> Philip Martin wrote on Mon, Mar 05, 2012 at 13:58:18 +0000:
>> Daniel Shahaf<da...@elego.de> writes:
>>
>>> Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
>>>> Daniel Shahaf<da...@elego.de> writes:
>>>>
>>>>> You're right, I misread the code: I mistakenly thought line 2867 will
>>>>> always re-read the revprop-gen file from disk. How about:
>>>>>
>>>>> Index: subversion/libsvn_fs_fs/fs_fs.c
>>>>> ===================================================================
>>>>> --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
>>>>> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
>>>>> @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
>>>>> fs_fs_data_t *ffd = fs->fsap_data;
>>>>> if (ffd->format>= SVN_FS_FS__MIN_PACKED_FORMAT)
>>>>> SVN_ERR(update_min_unpacked_rev(fs, pool));
>>>>> SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
>>>>> pool));
>>>>> + SVN_ERR(read_revprop_generation(fs, pool));
>>>>> err = body(baton, subpool);
>>>>> }
>>>>>
>>>> That looks like it works. But it only works if all writers update the
>>>> generation file so this whole caching scheme requires an FSFS format
>>>> bump to exclude 1.7 and earlier.
>>> +1
>>>
>>> (It was pointed on IRC, too.)
>> If we were to clear the cache after taking the write lock, either
>> unconditionally or if the revprop generation has changed, then we would
>> become compatible with pre-1.8 style writers.
>>
>> Of course this does highlight a change in svn_fs_t behaviour. A long
>> lived svn_fs_t may never see revprop updates as the cache never expires.
>> The cached values might get pushed out if the process (thead?) reads
>> other revisions, to get reasonably up-to-date values the caller must not
>> hold svn_fs_t objects too long.
> Which is a regression --- even a process that does proplist() in a loop
> should eventually see changes.
Hm. I would expect the live of a fs_t to be limited
by the life of the RA session. At least for our CL
client, this would not be a problem.
For TSVN dialogs like the repository browser that
keep client contexts open for a long time, it all
hinges on how long the RA session is kept open
by the SVN libs and whether the client has any
control over it.
Can anyone comment on that?
-- Stefan^2.
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 13:58:18 +0000:
> Daniel Shahaf <da...@elego.de> writes:
>
> > Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
> >> Daniel Shahaf <da...@elego.de> writes:
> >>
> >> > You're right, I misread the code: I mistakenly thought line 2867 will
> >> > always re-read the revprop-gen file from disk. How about:
> >> >
> >> > Index: subversion/libsvn_fs_fs/fs_fs.c
> >> > ===================================================================
> >> > --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
> >> > +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> >> > @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
> >> > fs_fs_data_t *ffd = fs->fsap_data;
> >> > if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
> >> > SVN_ERR(update_min_unpacked_rev(fs, pool));
> >> > SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
> >> > pool));
> >> > + SVN_ERR(read_revprop_generation(fs, pool));
> >> > err = body(baton, subpool);
> >> > }
> >> >
> >>
> >> That looks like it works. But it only works if all writers update the
> >> generation file so this whole caching scheme requires an FSFS format
> >> bump to exclude 1.7 and earlier.
> >
> > +1
> >
> > (It was pointed on IRC, too.)
>
> If we were to clear the cache after taking the write lock, either
> unconditionally or if the revprop generation has changed, then we would
> become compatible with pre-1.8 style writers.
>
> Of course this does highlight a change in svn_fs_t behaviour. A long
> lived svn_fs_t may never see revprop updates as the cache never expires.
> The cached values might get pushed out if the process (thead?) reads
> other revisions, to get reasonably up-to-date values the caller must not
> hold svn_fs_t objects too long.
Which is a regression --- even a process that does proplist() in a loop
should eventually see changes.
>
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 13:58:18 +0000:
> Daniel Shahaf <da...@elego.de> writes:
>
> > Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
> >> Daniel Shahaf <da...@elego.de> writes:
> >>
> >> > You're right, I misread the code: I mistakenly thought line 2867 will
> >> > always re-read the revprop-gen file from disk. How about:
> >> >
> >> > Index: subversion/libsvn_fs_fs/fs_fs.c
> >> > ===================================================================
> >> > --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
> >> > +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> >> > @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
> >> > fs_fs_data_t *ffd = fs->fsap_data;
> >> > if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
> >> > SVN_ERR(update_min_unpacked_rev(fs, pool));
> >> > SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
> >> > pool));
> >> > + SVN_ERR(read_revprop_generation(fs, pool));
> >> > err = body(baton, subpool);
> >> > }
> >> >
> >>
> >> That looks like it works. But it only works if all writers update the
> >> generation file so this whole caching scheme requires an FSFS format
> >> bump to exclude 1.7 and earlier.
> >
> > +1
> >
> > (It was pointed on IRC, too.)
>
> If we were to clear the cache after taking the write lock, either
> unconditionally or if the revprop generation has changed, then we would
> become compatible with pre-1.8 style writers.
>
> Of course this does highlight a change in svn_fs_t behaviour. A long
> lived svn_fs_t may never see revprop updates as the cache never expires.
> The cached values might get pushed out if the process (thead?) reads
> other revisions, to get reasonably up-to-date values the caller must not
> hold svn_fs_t objects too long.
Which is a regression --- even a process that does proplist() in a loop
should eventually see changes.
>
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
>> Daniel Shahaf <da...@elego.de> writes:
>>
>> > You're right, I misread the code: I mistakenly thought line 2867 will
>> > always re-read the revprop-gen file from disk. How about:
>> >
>> > Index: subversion/libsvn_fs_fs/fs_fs.c
>> > ===================================================================
>> > --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
>> > +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
>> > @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
>> > fs_fs_data_t *ffd = fs->fsap_data;
>> > if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
>> > SVN_ERR(update_min_unpacked_rev(fs, pool));
>> > SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
>> > pool));
>> > + SVN_ERR(read_revprop_generation(fs, pool));
>> > err = body(baton, subpool);
>> > }
>> >
>>
>> That looks like it works. But it only works if all writers update the
>> generation file so this whole caching scheme requires an FSFS format
>> bump to exclude 1.7 and earlier.
>
> +1
>
> (It was pointed on IRC, too.)
If we were to clear the cache after taking the write lock, either
unconditionally or if the revprop generation has changed, then we would
become compatible with pre-1.8 style writers.
Of course this does highlight a change in svn_fs_t behaviour. A long
lived svn_fs_t may never see revprop updates as the cache never expires.
The cached values might get pushed out if the process (thead?) reads
other revisions, to get reasonably up-to-date values the caller must not
hold svn_fs_t objects too long.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
>> Daniel Shahaf <da...@elego.de> writes:
>>
>> > You're right, I misread the code: I mistakenly thought line 2867 will
>> > always re-read the revprop-gen file from disk. How about:
>> >
>> > Index: subversion/libsvn_fs_fs/fs_fs.c
>> > ===================================================================
>> > --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
>> > +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
>> > @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
>> > fs_fs_data_t *ffd = fs->fsap_data;
>> > if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
>> > SVN_ERR(update_min_unpacked_rev(fs, pool));
>> > SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
>> > pool));
>> > + SVN_ERR(read_revprop_generation(fs, pool));
>> > err = body(baton, subpool);
>> > }
>> >
>>
>> That looks like it works. But it only works if all writers update the
>> generation file so this whole caching scheme requires an FSFS format
>> bump to exclude 1.7 and earlier.
>
> +1
>
> (It was pointed on IRC, too.)
If we were to clear the cache after taking the write lock, either
unconditionally or if the revprop generation has changed, then we would
become compatible with pre-1.8 style writers.
Of course this does highlight a change in svn_fs_t behaviour. A long
lived svn_fs_t may never see revprop updates as the cache never expires.
The cached values might get pushed out if the process (thead?) reads
other revisions, to get reasonably up-to-date values the caller must not
hold svn_fs_t objects too long.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
> Daniel Shahaf <da...@elego.de> writes:
>
> > You're right, I misread the code: I mistakenly thought line 2867 will
> > always re-read the revprop-gen file from disk. How about:
> >
> > Index: subversion/libsvn_fs_fs/fs_fs.c
> > ===================================================================
> > --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
> > +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> > @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
> > fs_fs_data_t *ffd = fs->fsap_data;
> > if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
> > SVN_ERR(update_min_unpacked_rev(fs, pool));
> > SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
> > pool));
> > + SVN_ERR(read_revprop_generation(fs, pool));
> > err = body(baton, subpool);
> > }
> >
>
> That looks like it works. But it only works if all writers update the
> generation file so this whole caching scheme requires an FSFS format
> bump to exclude 1.7 and earlier.
+1
(It was pointed on IRC, too.)
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 12:23:40 +0000:
> Daniel Shahaf <da...@elego.de> writes:
>
> > You're right, I misread the code: I mistakenly thought line 2867 will
> > always re-read the revprop-gen file from disk. How about:
> >
> > Index: subversion/libsvn_fs_fs/fs_fs.c
> > ===================================================================
> > --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
> > +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> > @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
> > fs_fs_data_t *ffd = fs->fsap_data;
> > if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
> > SVN_ERR(update_min_unpacked_rev(fs, pool));
> > SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
> > pool));
> > + SVN_ERR(read_revprop_generation(fs, pool));
> > err = body(baton, subpool);
> > }
> >
>
> That looks like it works. But it only works if all writers update the
> generation file so this whole caching scheme requires an FSFS format
> bump to exclude 1.7 and earlier.
+1
(It was pointed on IRC, too.)
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> You're right, I misread the code: I mistakenly thought line 2867 will
> always re-read the revprop-gen file from disk. How about:
>
> Index: subversion/libsvn_fs_fs/fs_fs.c
> ===================================================================
> --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
> fs_fs_data_t *ffd = fs->fsap_data;
> if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
> SVN_ERR(update_min_unpacked_rev(fs, pool));
> SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
> pool));
> + SVN_ERR(read_revprop_generation(fs, pool));
> err = body(baton, subpool);
> }
>
That looks like it works. But it only works if all writers update the
generation file so this whole caching scheme requires an FSFS format
bump to exclude 1.7 and earlier.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> You're right, I misread the code: I mistakenly thought line 2867 will
> always re-read the revprop-gen file from disk. How about:
>
> Index: subversion/libsvn_fs_fs/fs_fs.c
> ===================================================================
> --- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> @@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
> fs_fs_data_t *ffd = fs->fsap_data;
> if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
> SVN_ERR(update_min_unpacked_rev(fs, pool));
> SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
> pool));
> + SVN_ERR(read_revprop_generation(fs, pool));
> err = body(baton, subpool);
> }
>
That looks like it works. But it only works if all writers update the
generation file so this whole caching scheme requires an FSFS format
bump to exclude 1.7 and earlier.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 11:51:05 +0000:
> Daniel Shahaf <da...@elego.de> writes:
>
> > Philip Martin wrote on Mon, Mar 05, 2012 at 11:21:21 +0000:
> >> Daniel Shahaf <da...@elego.de> writes:
> >>
> >> >> What about atomic revprop changes? I don't see what ensures that the
> >> >> old value read by change_rev_prop_body is the most up-to-date value.
> >> >>
> >> >
> >> > ffd->revprop_generation is used as part of the cache key, the file is
> >> > written before the write-lock is released and read again as soon as the
> >> > write-lock is taken. Do you see a problem?
> >>
> >> I'm worried about the case where the FS passed to
> >> svn_fs_fs__change_rev_prop has a cache that is already populated. The
> >> values in the cache might be out-of-date because some other thread with
> >> another FS has written newer values. It looks like change_rev_prop_body
> >> will examine the out-of-date cached value when doing the "atomic"
> >> update.
> >
> > The values will be in a cache under a key of the form (N,g), but the
> > cache lookup will use a key (N,g'), where $g' > g$, and will therefore
> > fail. Here, $N$ is a revnum and $g$ is a revprop generation.
>
> Suppose the cache gets populated in the atomic change process with
> revprop generation g. Some other process writes to the repository
> creating revprop generation g' making the cache in the atomic process
> out-of-date. Now the atomic process takes the write lock and does the
> "atomic" revprop change using the out-of-date cache.
You're right, I misread the code: I mistakenly thought line 2867 will
always re-read the revprop-gen file from disk. How about:
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
fs_fs_data_t *ffd = fs->fsap_data;
if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
SVN_ERR(update_min_unpacked_rev(fs, pool));
SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
pool));
+ SVN_ERR(read_revprop_generation(fs, pool));
err = body(baton, subpool);
}
(And as an aside, perhaps the return statement in
check_revprop_generation() should have been written more clearly.)
>
> Without the cache change_rev_prop_body would return
> SVN_ERR_FS_PROP_BASEVALUE_MISMATCH if the current value doesn't match
> the supplied value. The cache means the current value might be
> out-of-date.
>
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 11:51:05 +0000:
> Daniel Shahaf <da...@elego.de> writes:
>
> > Philip Martin wrote on Mon, Mar 05, 2012 at 11:21:21 +0000:
> >> Daniel Shahaf <da...@elego.de> writes:
> >>
> >> >> What about atomic revprop changes? I don't see what ensures that the
> >> >> old value read by change_rev_prop_body is the most up-to-date value.
> >> >>
> >> >
> >> > ffd->revprop_generation is used as part of the cache key, the file is
> >> > written before the write-lock is released and read again as soon as the
> >> > write-lock is taken. Do you see a problem?
> >>
> >> I'm worried about the case where the FS passed to
> >> svn_fs_fs__change_rev_prop has a cache that is already populated. The
> >> values in the cache might be out-of-date because some other thread with
> >> another FS has written newer values. It looks like change_rev_prop_body
> >> will examine the out-of-date cached value when doing the "atomic"
> >> update.
> >
> > The values will be in a cache under a key of the form (N,g), but the
> > cache lookup will use a key (N,g'), where $g' > g$, and will therefore
> > fail. Here, $N$ is a revnum and $g$ is a revprop generation.
>
> Suppose the cache gets populated in the atomic change process with
> revprop generation g. Some other process writes to the repository
> creating revprop generation g' making the cache in the atomic process
> out-of-date. Now the atomic process takes the write lock and does the
> "atomic" revprop change using the out-of-date cache.
You're right, I misread the code: I mistakenly thought line 2867 will
always re-read the revprop-gen file from disk. How about:
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 1297002)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -583,8 +583,9 @@ with_some_lock_file(svn_fs_t *fs,
fs_fs_data_t *ffd = fs->fsap_data;
if (ffd->format >= SVN_FS_FS__MIN_PACKED_FORMAT)
SVN_ERR(update_min_unpacked_rev(fs, pool));
SVN_ERR(get_youngest(&ffd->youngest_rev_cache, fs->path,
pool));
+ SVN_ERR(read_revprop_generation(fs, pool));
err = body(baton, subpool);
}
(And as an aside, perhaps the return statement in
check_revprop_generation() should have been written more clearly.)
>
> Without the cache change_rev_prop_body would return
> SVN_ERR_FS_PROP_BASEVALUE_MISMATCH if the current value doesn't match
> the supplied value. The cache means the current value might be
> out-of-date.
>
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> Without the cache change_rev_prop_body would return
> SVN_ERR_FS_PROP_BASEVALUE_MISMATCH if the current value doesn't match
> the supplied value. The cache means the current value might be
> out-of-date.
I can demonstrate a race. Set log to 'xx'
$ svn ps --revprop -r3 svn:log xx file://`pwd`/repo
Start a change to 'yy':
$ gdb -arg svn ps --revprop -r3 svn:log yy file://`pwd`/repo
(gdb) b svn_fs_fs__change_rev_prop
(gdb) r
breakpoint 1
Racing change to 'zz':
$ svn ps --revprop -r3 svn:log zz file://`pwd`/repo
Continue change to 'yy'
(gdb) b change_rev_prop_body
(gdb) c
breakpoint 2
(gbd) n
(gbd) n
7683 if (cb->old_value_p)
(gdb) p cb->old_value_p[0].data
$2 = 0x68bbf8 "xx"
The "atomic" change is being done based on the ood value.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> Without the cache change_rev_prop_body would return
> SVN_ERR_FS_PROP_BASEVALUE_MISMATCH if the current value doesn't match
> the supplied value. The cache means the current value might be
> out-of-date.
I can demonstrate a race. Set log to 'xx'
$ svn ps --revprop -r3 svn:log xx file://`pwd`/repo
Start a change to 'yy':
$ gdb -arg svn ps --revprop -r3 svn:log yy file://`pwd`/repo
(gdb) b svn_fs_fs__change_rev_prop
(gdb) r
breakpoint 1
Racing change to 'zz':
$ svn ps --revprop -r3 svn:log zz file://`pwd`/repo
Continue change to 'yy'
(gdb) b change_rev_prop_body
(gdb) c
breakpoint 2
(gbd) n
(gbd) n
7683 if (cb->old_value_p)
(gdb) p cb->old_value_p[0].data
$2 = 0x68bbf8 "xx"
The "atomic" change is being done based on the ood value.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> Philip Martin wrote on Mon, Mar 05, 2012 at 11:21:21 +0000:
>> Daniel Shahaf <da...@elego.de> writes:
>>
>> >> What about atomic revprop changes? I don't see what ensures that the
>> >> old value read by change_rev_prop_body is the most up-to-date value.
>> >>
>> >
>> > ffd->revprop_generation is used as part of the cache key, the file is
>> > written before the write-lock is released and read again as soon as the
>> > write-lock is taken. Do you see a problem?
>>
>> I'm worried about the case where the FS passed to
>> svn_fs_fs__change_rev_prop has a cache that is already populated. The
>> values in the cache might be out-of-date because some other thread with
>> another FS has written newer values. It looks like change_rev_prop_body
>> will examine the out-of-date cached value when doing the "atomic"
>> update.
>
> The values will be in a cache under a key of the form (N,g), but the
> cache lookup will use a key (N,g'), where $g' > g$, and will therefore
> fail. Here, $N$ is a revnum and $g$ is a revprop generation.
Suppose the cache gets populated in the atomic change process with
revprop generation g. Some other process writes to the repository
creating revprop generation g' making the cache in the atomic process
out-of-date. Now the atomic process takes the write lock and does the
"atomic" revprop change using the out-of-date cache.
Without the cache change_rev_prop_body would return
SVN_ERR_FS_PROP_BASEVALUE_MISMATCH if the current value doesn't match
the supplied value. The cache means the current value might be
out-of-date.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> Philip Martin wrote on Mon, Mar 05, 2012 at 11:21:21 +0000:
>> Daniel Shahaf <da...@elego.de> writes:
>>
>> >> What about atomic revprop changes? I don't see what ensures that the
>> >> old value read by change_rev_prop_body is the most up-to-date value.
>> >>
>> >
>> > ffd->revprop_generation is used as part of the cache key, the file is
>> > written before the write-lock is released and read again as soon as the
>> > write-lock is taken. Do you see a problem?
>>
>> I'm worried about the case where the FS passed to
>> svn_fs_fs__change_rev_prop has a cache that is already populated. The
>> values in the cache might be out-of-date because some other thread with
>> another FS has written newer values. It looks like change_rev_prop_body
>> will examine the out-of-date cached value when doing the "atomic"
>> update.
>
> The values will be in a cache under a key of the form (N,g), but the
> cache lookup will use a key (N,g'), where $g' > g$, and will therefore
> fail. Here, $N$ is a revnum and $g$ is a revprop generation.
Suppose the cache gets populated in the atomic change process with
revprop generation g. Some other process writes to the repository
creating revprop generation g' making the cache in the atomic process
out-of-date. Now the atomic process takes the write lock and does the
"atomic" revprop change using the out-of-date cache.
Without the cache change_rev_prop_body would return
SVN_ERR_FS_PROP_BASEVALUE_MISMATCH if the current value doesn't match
the supplied value. The cache means the current value might be
out-of-date.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 11:21:21 +0000:
> Daniel Shahaf <da...@elego.de> writes:
>
> >> What about atomic revprop changes? I don't see what ensures that the
> >> old value read by change_rev_prop_body is the most up-to-date value.
> >>
> >
> > ffd->revprop_generation is used as part of the cache key, the file is
> > written before the write-lock is released and read again as soon as the
> > write-lock is taken. Do you see a problem?
>
> I'm worried about the case where the FS passed to
> svn_fs_fs__change_rev_prop has a cache that is already populated. The
> values in the cache might be out-of-date because some other thread with
> another FS has written newer values. It looks like change_rev_prop_body
> will examine the out-of-date cached value when doing the "atomic"
> update.
The values will be in a cache under a key of the form (N,g), but the
cache lookup will use a key (N,g'), where $g' > g$, and will therefore
fail. Here, $N$ is a revnum and $g$ is a revprop generation.
>
> --
> Philip
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> Daniel Shahaf <da...@elego.de> writes:
>
>>> What about atomic revprop changes? I don't see what ensures that the
>>> old value read by change_rev_prop_body is the most up-to-date value.
>>>
>>
>> ffd->revprop_generation is used as part of the cache key, the file is
>> written before the write-lock is released and read again as soon as the
>> write-lock is taken. Do you see a problem?
>
> I'm worried about the case where the FS passed to
> svn_fs_fs__change_rev_prop has a cache that is already populated. The
> values in the cache might be out-of-date because some other thread with
> another FS has written newer values. It looks like change_rev_prop_body
> will examine the out-of-date cached value when doing the "atomic"
> update.
Maybe the threaded case is OK because there is only one cache? What
about another process that changes the repository after the cache has
been populated?
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> Daniel Shahaf <da...@elego.de> writes:
>
>>> What about atomic revprop changes? I don't see what ensures that the
>>> old value read by change_rev_prop_body is the most up-to-date value.
>>>
>>
>> ffd->revprop_generation is used as part of the cache key, the file is
>> written before the write-lock is released and read again as soon as the
>> write-lock is taken. Do you see a problem?
>
> I'm worried about the case where the FS passed to
> svn_fs_fs__change_rev_prop has a cache that is already populated. The
> values in the cache might be out-of-date because some other thread with
> another FS has written newer values. It looks like change_rev_prop_body
> will examine the out-of-date cached value when doing the "atomic"
> update.
Maybe the threaded case is OK because there is only one cache? What
about another process that changes the repository after the cache has
been populated?
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 11:21:21 +0000:
> Daniel Shahaf <da...@elego.de> writes:
>
> >> What about atomic revprop changes? I don't see what ensures that the
> >> old value read by change_rev_prop_body is the most up-to-date value.
> >>
> >
> > ffd->revprop_generation is used as part of the cache key, the file is
> > written before the write-lock is released and read again as soon as the
> > write-lock is taken. Do you see a problem?
>
> I'm worried about the case where the FS passed to
> svn_fs_fs__change_rev_prop has a cache that is already populated. The
> values in the cache might be out-of-date because some other thread with
> another FS has written newer values. It looks like change_rev_prop_body
> will examine the out-of-date cached value when doing the "atomic"
> update.
The values will be in a cache under a key of the form (N,g), but the
cache lookup will use a key (N,g'), where $g' > g$, and will therefore
fail. Here, $N$ is a revnum and $g$ is a revprop generation.
>
> --
> Philip
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
>> What about atomic revprop changes? I don't see what ensures that the
>> old value read by change_rev_prop_body is the most up-to-date value.
>>
>
> ffd->revprop_generation is used as part of the cache key, the file is
> written before the write-lock is released and read again as soon as the
> write-lock is taken. Do you see a problem?
I'm worried about the case where the FS passed to
svn_fs_fs__change_rev_prop has a cache that is already populated. The
values in the cache might be out-of-date because some other thread with
another FS has written newer values. It looks like change_rev_prop_body
will examine the out-of-date cached value when doing the "atomic"
update.
--
Philip
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
>> What about atomic revprop changes? I don't see what ensures that the
>> old value read by change_rev_prop_body is the most up-to-date value.
>>
>
> ffd->revprop_generation is used as part of the cache key, the file is
> written before the write-lock is released and read again as soon as the
> write-lock is taken. Do you see a problem?
I'm worried about the case where the FS passed to
svn_fs_fs__change_rev_prop has a cache that is already populated. The
values in the cache might be out-of-date because some other thread with
another FS has written newer values. It looks like change_rev_prop_body
will examine the out-of-date cached value when doing the "atomic"
update.
--
Philip
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 10:50:29 +0000:
> Hyrum K Wright <hy...@wandisco.com> writes:
>
> > On Sat, Mar 3, 2012 at 5:31 AM, <st...@apache.org> wrote:
> >> Author: stefan2
> >> Date: Sat Mar 3 11:31:17 2012
> >> New Revision: 1296604
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
> >> Log:
> >> Certain operations, e.g. svn ls, will contain timestamp and author
> >> information from many different revisions. A list of all projects
> >> in the root of the wordpress repository will open, read and close
> >>>75.000 revision property files (3 reads for each list entry)
> >>
> >> This commit implements revprop caching. It will be activated as
> >> part of the full-text caching option.
> >>
> >> Since revprops may be written by other threads or processes, we
> >> need to track the revprop changes. A new special file contains a
> >> counter that will be increased each time revision properties get
> >> rewritten.
> >>
> >> This counter is internally called "revprop generation" and will be
> >> read upon the first revprop access for given fs_t. Later changes
> >> may remain invisible for that fs_t. This behavior is in line with
> >> our revprop handling in other parts of FS_FS. If a revprop gets
> >> rewritten, the fs_t doing the write will use the new generation
> >> from that point on and will thus see all caches up to and including
> >> its own.
> >>
> >> Since the revprop generation becomes part of the cache key, each
> >> fs_t will only see revprops from its generation. It may also
> >> create new cache entries tagged with that generation, i.e. those
> >> would appear to be outdated for newer fs_t. But that will simply
> >> cause a benign false negative upon lookup. No fs_t will see
> >> data that got replaced before that fs_t was created.
> >
> > How does this potentially interact with revprop packing?
>
> What about atomic revprop changes? I don't see what ensures that the
> old value read by change_rev_prop_body is the most up-to-date value.
>
ffd->revprop_generation is used as part of the cache key, the file is
written before the write-lock is released and read again as soon as the
write-lock is taken. Do you see a problem?
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Mon, Mar 05, 2012 at 10:50:29 +0000:
> Hyrum K Wright <hy...@wandisco.com> writes:
>
> > On Sat, Mar 3, 2012 at 5:31 AM, <st...@apache.org> wrote:
> >> Author: stefan2
> >> Date: Sat Mar 3 11:31:17 2012
> >> New Revision: 1296604
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
> >> Log:
> >> Certain operations, e.g. svn ls, will contain timestamp and author
> >> information from many different revisions. A list of all projects
> >> in the root of the wordpress repository will open, read and close
> >>>75.000 revision property files (3 reads for each list entry)
> >>
> >> This commit implements revprop caching. It will be activated as
> >> part of the full-text caching option.
> >>
> >> Since revprops may be written by other threads or processes, we
> >> need to track the revprop changes. A new special file contains a
> >> counter that will be increased each time revision properties get
> >> rewritten.
> >>
> >> This counter is internally called "revprop generation" and will be
> >> read upon the first revprop access for given fs_t. Later changes
> >> may remain invisible for that fs_t. This behavior is in line with
> >> our revprop handling in other parts of FS_FS. If a revprop gets
> >> rewritten, the fs_t doing the write will use the new generation
> >> from that point on and will thus see all caches up to and including
> >> its own.
> >>
> >> Since the revprop generation becomes part of the cache key, each
> >> fs_t will only see revprops from its generation. It may also
> >> create new cache entries tagged with that generation, i.e. those
> >> would appear to be outdated for newer fs_t. But that will simply
> >> cause a benign false negative upon lookup. No fs_t will see
> >> data that got replaced before that fs_t was created.
> >
> > How does this potentially interact with revprop packing?
>
> What about atomic revprop changes? I don't see what ensures that the
> old value read by change_rev_prop_body is the most up-to-date value.
>
ffd->revprop_generation is used as part of the cache key, the file is
written before the write-lock is released and read again as soon as the
write-lock is taken. Do you see a problem?
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Hyrum K Wright <hy...@wandisco.com> writes:
> On Sat, Mar 3, 2012 at 5:31 AM, <st...@apache.org> wrote:
>> Author: stefan2
>> Date: Sat Mar 3 11:31:17 2012
>> New Revision: 1296604
>>
>> URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
>> Log:
>> Certain operations, e.g. svn ls, will contain timestamp and author
>> information from many different revisions. A list of all projects
>> in the root of the wordpress repository will open, read and close
>>>75.000 revision property files (3 reads for each list entry)
>>
>> This commit implements revprop caching. It will be activated as
>> part of the full-text caching option.
>>
>> Since revprops may be written by other threads or processes, we
>> need to track the revprop changes. A new special file contains a
>> counter that will be increased each time revision properties get
>> rewritten.
>>
>> This counter is internally called "revprop generation" and will be
>> read upon the first revprop access for given fs_t. Later changes
>> may remain invisible for that fs_t. This behavior is in line with
>> our revprop handling in other parts of FS_FS. If a revprop gets
>> rewritten, the fs_t doing the write will use the new generation
>> from that point on and will thus see all caches up to and including
>> its own.
>>
>> Since the revprop generation becomes part of the cache key, each
>> fs_t will only see revprops from its generation. It may also
>> create new cache entries tagged with that generation, i.e. those
>> would appear to be outdated for newer fs_t. But that will simply
>> cause a benign false negative upon lookup. No fs_t will see
>> data that got replaced before that fs_t was created.
>
> How does this potentially interact with revprop packing?
What about atomic revprop changes? I don't see what ensures that the
old value read by change_rev_prop_body is the most up-to-date value.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Philip Martin <ph...@wandisco.com>.
Hyrum K Wright <hy...@wandisco.com> writes:
> On Sat, Mar 3, 2012 at 5:31 AM, <st...@apache.org> wrote:
>> Author: stefan2
>> Date: Sat Mar 3 11:31:17 2012
>> New Revision: 1296604
>>
>> URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
>> Log:
>> Certain operations, e.g. svn ls, will contain timestamp and author
>> information from many different revisions. A list of all projects
>> in the root of the wordpress repository will open, read and close
>>>75.000 revision property files (3 reads for each list entry)
>>
>> This commit implements revprop caching. It will be activated as
>> part of the full-text caching option.
>>
>> Since revprops may be written by other threads or processes, we
>> need to track the revprop changes. A new special file contains a
>> counter that will be increased each time revision properties get
>> rewritten.
>>
>> This counter is internally called "revprop generation" and will be
>> read upon the first revprop access for given fs_t. Later changes
>> may remain invisible for that fs_t. This behavior is in line with
>> our revprop handling in other parts of FS_FS. If a revprop gets
>> rewritten, the fs_t doing the write will use the new generation
>> from that point on and will thus see all caches up to and including
>> its own.
>>
>> Since the revprop generation becomes part of the cache key, each
>> fs_t will only see revprops from its generation. It may also
>> create new cache entries tagged with that generation, i.e. those
>> would appear to be outdated for newer fs_t. But that will simply
>> cause a benign false negative upon lookup. No fs_t will see
>> data that got replaced before that fs_t was created.
>
> How does this potentially interact with revprop packing?
What about atomic revprop changes? I don't see what ensures that the
old value read by change_rev_prop_body is the most up-to-date value.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs:
caching.c fs.h fs_fs.c
Posted by Stefan Fuhrmann <eq...@web.de>.
On 04.03.2012 11:06, Daniel Shahaf wrote:
> Hyrum K Wright wrote on Sat, Mar 03, 2012 at 09:51:07 -0600:
>> On Sat, Mar 3, 2012 at 5:31 AM,<st...@apache.org> wrote:
>>> Author: stefan2
>>> Date: Sat Mar 3 11:31:17 2012
>>> New Revision: 1296604
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
>>> Log:
>>> Certain operations, e.g. svn ls, will contain timestamp and author
>>> information from many different revisions. A list of all projects
>>> in the root of the wordpress repository will open, read and close
>>>> 75.000 revision property files (3 reads for each list entry)
>>> This commit implements revprop caching. It will be activated as
>>> part of the full-text caching option.
>>>
>>> Since revprops may be written by other threads or processes, we
>>> need to track the revprop changes. A new special file contains a
>>> counter that will be increased each time revision properties get
>>> rewritten.
>>>
>>> This counter is internally called "revprop generation" and will be
>>> read upon the first revprop access for given fs_t. Later changes
>>> may remain invisible for that fs_t. This behavior is in line with
>>> our revprop handling in other parts of FS_FS. If a revprop gets
>>> rewritten, the fs_t doing the write will use the new generation
>>> from that point on and will thus see all caches up to and including
>>> its own.
>>>
>>> Since the revprop generation becomes part of the cache key, each
>>> fs_t will only see revprops from its generation. It may also
>>> create new cache entries tagged with that generation, i.e. those
>>> would appear to be outdated for newer fs_t. But that will simply
>>> cause a benign false negative upon lookup. No fs_t will see
>>> data that got replaced before that fs_t was created.
>> How does this potentially interact with revprop packing?
>>
> I believe the two features are orthogonal.
Yes, they can be treated as being independent --
apart from the occasional merge conflict. Revprop
caching simply hooks the revprop read and write
functions.
Since packed revprops come as larger files, a naive
implementation (read revprop -> open packed file,
read just one revprop, close file) would be even
slower than on trunk, some sort of caching is in
order keep performance acceptable.
Revprop caching could come in quite handy here:
read packed file, parse & cache *all* revpros therein,
close file. Most redundant reads will now be eliminated.
-- Stefan^2.
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Hyrum K Wright wrote on Sat, Mar 03, 2012 at 09:51:07 -0600:
> On Sat, Mar 3, 2012 at 5:31 AM, <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Sat Mar 3 11:31:17 2012
> > New Revision: 1296604
> >
> > URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
> > Log:
> > Certain operations, e.g. svn ls, will contain timestamp and author
> > information from many different revisions. A list of all projects
> > in the root of the wordpress repository will open, read and close
> >>75.000 revision property files (3 reads for each list entry)
> >
> > This commit implements revprop caching. It will be activated as
> > part of the full-text caching option.
> >
> > Since revprops may be written by other threads or processes, we
> > need to track the revprop changes. A new special file contains a
> > counter that will be increased each time revision properties get
> > rewritten.
> >
> > This counter is internally called "revprop generation" and will be
> > read upon the first revprop access for given fs_t. Later changes
> > may remain invisible for that fs_t. This behavior is in line with
> > our revprop handling in other parts of FS_FS. If a revprop gets
> > rewritten, the fs_t doing the write will use the new generation
> > from that point on and will thus see all caches up to and including
> > its own.
> >
> > Since the revprop generation becomes part of the cache key, each
> > fs_t will only see revprops from its generation. It may also
> > create new cache entries tagged with that generation, i.e. those
> > would appear to be outdated for newer fs_t. But that will simply
> > cause a benign false negative upon lookup. No fs_t will see
> > data that got replaced before that fs_t was created.
>
> How does this potentially interact with revprop packing?
>
I believe the two features are orthogonal.
> -Hyrum
>
> --
>
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com/
Re: svn commit: r1296604 - in
/subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c
Posted by Daniel Shahaf <da...@elego.de>.
Hyrum K Wright wrote on Sat, Mar 03, 2012 at 09:51:07 -0600:
> On Sat, Mar 3, 2012 at 5:31 AM, <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Sat Mar 3 11:31:17 2012
> > New Revision: 1296604
> >
> > URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
> > Log:
> > Certain operations, e.g. svn ls, will contain timestamp and author
> > information from many different revisions. A list of all projects
> > in the root of the wordpress repository will open, read and close
> >>75.000 revision property files (3 reads for each list entry)
> >
> > This commit implements revprop caching. It will be activated as
> > part of the full-text caching option.
> >
> > Since revprops may be written by other threads or processes, we
> > need to track the revprop changes. A new special file contains a
> > counter that will be increased each time revision properties get
> > rewritten.
> >
> > This counter is internally called "revprop generation" and will be
> > read upon the first revprop access for given fs_t. Later changes
> > may remain invisible for that fs_t. This behavior is in line with
> > our revprop handling in other parts of FS_FS. If a revprop gets
> > rewritten, the fs_t doing the write will use the new generation
> > from that point on and will thus see all caches up to and including
> > its own.
> >
> > Since the revprop generation becomes part of the cache key, each
> > fs_t will only see revprops from its generation. It may also
> > create new cache entries tagged with that generation, i.e. those
> > would appear to be outdated for newer fs_t. But that will simply
> > cause a benign false negative upon lookup. No fs_t will see
> > data that got replaced before that fs_t was created.
>
> How does this potentially interact with revprop packing?
>
I believe the two features are orthogonal.
> -Hyrum
>
> --
>
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com/
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs:
caching.c fs.h fs_fs.c
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Sat, Mar 3, 2012 at 5:31 AM, <st...@apache.org> wrote:
> Author: stefan2
> Date: Sat Mar 3 11:31:17 2012
> New Revision: 1296604
>
> URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
> Log:
> Certain operations, e.g. svn ls, will contain timestamp and author
> information from many different revisions. A list of all projects
> in the root of the wordpress repository will open, read and close
>>75.000 revision property files (3 reads for each list entry)
>
> This commit implements revprop caching. It will be activated as
> part of the full-text caching option.
>
> Since revprops may be written by other threads or processes, we
> need to track the revprop changes. A new special file contains a
> counter that will be increased each time revision properties get
> rewritten.
>
> This counter is internally called "revprop generation" and will be
> read upon the first revprop access for given fs_t. Later changes
> may remain invisible for that fs_t. This behavior is in line with
> our revprop handling in other parts of FS_FS. If a revprop gets
> rewritten, the fs_t doing the write will use the new generation
> from that point on and will thus see all caches up to and including
> its own.
>
> Since the revprop generation becomes part of the cache key, each
> fs_t will only see revprops from its generation. It may also
> create new cache entries tagged with that generation, i.e. those
> would appear to be outdated for newer fs_t. But that will simply
> cause a benign false negative upon lookup. No fs_t will see
> data that got replaced before that fs_t was created.
How does this potentially interact with revprop packing?
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1296604 - in /subversion/trunk/subversion/libsvn_fs_fs:
caching.c fs.h fs_fs.c
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Sat, Mar 3, 2012 at 5:31 AM, <st...@apache.org> wrote:
> Author: stefan2
> Date: Sat Mar 3 11:31:17 2012
> New Revision: 1296604
>
> URL: http://svn.apache.org/viewvc?rev=1296604&view=rev
> Log:
> Certain operations, e.g. svn ls, will contain timestamp and author
> information from many different revisions. A list of all projects
> in the root of the wordpress repository will open, read and close
>>75.000 revision property files (3 reads for each list entry)
>
> This commit implements revprop caching. It will be activated as
> part of the full-text caching option.
>
> Since revprops may be written by other threads or processes, we
> need to track the revprop changes. A new special file contains a
> counter that will be increased each time revision properties get
> rewritten.
>
> This counter is internally called "revprop generation" and will be
> read upon the first revprop access for given fs_t. Later changes
> may remain invisible for that fs_t. This behavior is in line with
> our revprop handling in other parts of FS_FS. If a revprop gets
> rewritten, the fs_t doing the write will use the new generation
> from that point on and will thus see all caches up to and including
> its own.
>
> Since the revprop generation becomes part of the cache key, each
> fs_t will only see revprops from its generation. It may also
> create new cache entries tagged with that generation, i.e. those
> would appear to be outdated for newer fs_t. But that will simply
> cause a benign false negative upon lookup. No fs_t will see
> data that got replaced before that fs_t was created.
How does this potentially interact with revprop packing?
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/