You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2012/04/28 05:45:12 UTC
Tracebacks in svnadmin tests (fs->uuid issues)
Hey Daniel,
I had four svnadmin tests dump core due to an assertion: 8, 21, 26,
27. These are all marked XFail(), so my test run "passed".
The stack traces on all four look like this:
#4 0x000f4549 in svn_error__malfunction (can_return=1, file=0xa91bd
"subversion/libsvn_fs_fs/fs.c", line=76, expr=0xa91b4 "fs->uuid") at
subversion/libsvn_subr/error.c:698
#5 0x00081455 in fs_serialized_init (fs=0x817f58,
common_pool=0x80fa18, pool=0x80fa18) at
subversion/libsvn_fs_fs/fs.c:76
#6 0x00082040 in fs_hotcopy (src_fs=0x817f30, dst_fs=0x817f58,
src_path=0x810028
"/Users/gstein/src/svn/subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-8/db",
dst_path=0x816a60
"/Volumes/untitled/svn-test-work/repositories/svnadmin_tests-8.backup/db",
clean_logs=0, incremental=0, cancel_func=0x240f <check_cancel>,
cancel_baton=0x0, pool=0x80fa18) at subversion/libsvn_fs_fs/fs.c:321
#7 0x00014337 in svn_fs_hotcopy2 (src_path=0x810028
"/Users/gstein/src/svn/subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-8/db",
dst_path=0x816a60
"/Volumes/untitled/svn-test-work/repositories/svnadmin_tests-8.backup/db",
clean=0, incremental=0, cancel_func=0x240f <check_cancel>,
cancel_baton=0x0, scratch_pool=0x80fa18) at
subversion/libsvn_fs/fs-loader.c:497
#8 0x0005f8dd in svn_repos_hotcopy2 (src_path=0x80fc48
"/Users/gstein/src/svn/subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-8",
dst_path=0x80fce8 "", clean_logs=0, incremental=0, cancel_func=0x240f
<check_cancel>, cancel_baton=0x0, pool=0x80fa18) at
subversion/libsvn_repos/repos.c:1894
#9 0x000052d7 in subcommand_hotcopy (os=0x80fb38, baton=0xbfffea7c,
pool=0x80fa18) at subversion/svnadmin/main.c:1455
#10 0x000075bf in main (argc=4, argv=0xbfffeb74) at
subversion/svnadmin/main.c:2101
The function calls are the same across the four. I have not examined
them for line number or argument similarity, but just assumed all four
had the same pattern.
Cheers,
-g
Re: Tracebacks in svnadmin tests (fs->uuid issues)
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> I'm confused. How the heck does putting a UUID into svn_fs_t result in such
> a fallout? New files!? This sounds brittle and poor encapsulation. It
> seemed like an easy problem. Why is it rippling into such a large issue?
I think it has always been a problem if one process accessed two
separate repositories with the same UUID. So it's an existing problem
exposed by the new hotcopy2 implemenation and the uuid change.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: Tracebacks in svnadmin tests (fs->uuid issues)
Posted by Daniel Shahaf <da...@elego.de>.
Greg Stein wrote on Sun, Apr 29, 2012 at 03:32:51 -0400:
> On Apr 29, 2012 2:51 AM, "Daniel Shahaf" <da...@elego.de> wrote:
> >
> > Daniel Shahaf wrote on Sat, Apr 28, 2012 at 11:27:37 +0300:
> > > Greg Stein wrote on Fri, Apr 27, 2012 at 23:45:12 -0400:
> > > > Hey Daniel,
> > > >
> > > > I had four svnadmin tests dump core due to an assertion: 8, 21, 26,
> > > > 27. These are all marked XFail(), so my test run "passed".
> > >
> > > AFAIK all of those are due to the same issue: dst_fs->uuid not being
> > > initialized. Philip noted that there are probably worse problems here,
> > > since fs_fs_shared_data_t (ffd->ffsd) holds various locks and caches
> that
> > > are indexed by UUID only.
> >
> > We store the following data for use by >1 svn_fs_t object, where today
> > the sharing is keyed on UUID only:
> >
> > typedef struct fs_fs_shared_data_t {
> > fs_fs_shared_txn_data_t *txns;
> > fs_fs_shared_txn_data_t *free_txn;
> > svn_mutex__t *txn_list_lock;
> > svn_mutex__t *fs_write_lock;
> > svn_mutex__t *txn_current_lock;
> > apr_pool_t *common_pool;
> > } fs_fs_shared_data_t;
> >
> > Hotcopy needs to support ≥2 filesystem instances which have the same
> > contents and UUID. (Keeping the UUID is part of the API.) However,
> > everything above except common_pool needs to be per-instance. So, we
> > need some sort of per-instance identifier to use as the cache key.
> >
> > So, for example, one way to proceed would be to maintain
> > a ${REPOS}/db/uuid2 file, which is not exposed to fs-loader.h, and which
> > is used as part of the key to ffd->ffsd structures.
> >
> > Assuming we agree on the course, I should be able to get this
> > implemented this week.
>
> I'm confused. How the heck does putting a UUID into svn_fs_t result in such
> a fallout? New files!? This sounds brittle and poor encapsulation. It
> seemed like an easy problem. Why is it rippling into such a large issue?
fs_fs_data_t is allocated by apr_pcalloc(). svn_fs_t is allocated by
apr_palloc(). Moving the uuid field from the former to the latter
exposed a problem where it was passed in fs_serialized_init() to
apr_pstrcat() before being set.
The problematic caller is part of the new-in-1.8.0 'incremental hotcopy'
feature, however the underlying issue is as old as 1.5.x:
http://svn.apache.org/viewvc/subversion/tags/1.5.0/subversion/libsvn_fs_fs/fs.h?view=markup#l134
Re: Tracebacks in svnadmin tests (fs->uuid issues)
Posted by Greg Stein <gs...@gmail.com>.
On Apr 29, 2012 2:51 AM, "Daniel Shahaf" <da...@elego.de> wrote:
>
> Daniel Shahaf wrote on Sat, Apr 28, 2012 at 11:27:37 +0300:
> > Greg Stein wrote on Fri, Apr 27, 2012 at 23:45:12 -0400:
> > > Hey Daniel,
> > >
> > > I had four svnadmin tests dump core due to an assertion: 8, 21, 26,
> > > 27. These are all marked XFail(), so my test run "passed".
> >
> > AFAIK all of those are due to the same issue: dst_fs->uuid not being
> > initialized. Philip noted that there are probably worse problems here,
> > since fs_fs_shared_data_t (ffd->ffsd) holds various locks and caches
that
> > are indexed by UUID only.
>
> We store the following data for use by >1 svn_fs_t object, where today
> the sharing is keyed on UUID only:
>
> typedef struct fs_fs_shared_data_t {
> fs_fs_shared_txn_data_t *txns;
> fs_fs_shared_txn_data_t *free_txn;
> svn_mutex__t *txn_list_lock;
> svn_mutex__t *fs_write_lock;
> svn_mutex__t *txn_current_lock;
> apr_pool_t *common_pool;
> } fs_fs_shared_data_t;
>
> Hotcopy needs to support ≥2 filesystem instances which have the same
> contents and UUID. (Keeping the UUID is part of the API.) However,
> everything above except common_pool needs to be per-instance. So, we
> need some sort of per-instance identifier to use as the cache key.
>
> So, for example, one way to proceed would be to maintain
> a ${REPOS}/db/uuid2 file, which is not exposed to fs-loader.h, and which
> is used as part of the key to ffd->ffsd structures.
>
> Assuming we agree on the course, I should be able to get this
> implemented this week.
I'm confused. How the heck does putting a UUID into svn_fs_t result in such
a fallout? New files!? This sounds brittle and poor encapsulation. It
seemed like an easy problem. Why is it rippling into such a large issue?
-g
Re: Tracebacks in svnadmin tests (fs->uuid issues)
Posted by Daniel Shahaf <da...@elego.de>.
Daniel Shahaf wrote on Sat, Apr 28, 2012 at 11:27:37 +0300:
> Greg Stein wrote on Fri, Apr 27, 2012 at 23:45:12 -0400:
> > Hey Daniel,
> >
> > I had four svnadmin tests dump core due to an assertion: 8, 21, 26,
> > 27. These are all marked XFail(), so my test run "passed".
>
> AFAIK all of those are due to the same issue: dst_fs->uuid not being
> initialized. Philip noted that there are probably worse problems here,
> since fs_fs_shared_data_t (ffd->ffsd) holds various locks and caches that
> are indexed by UUID only.
We store the following data for use by >1 svn_fs_t object, where today
the sharing is keyed on UUID only:
typedef struct fs_fs_shared_data_t {
fs_fs_shared_txn_data_t *txns;
fs_fs_shared_txn_data_t *free_txn;
svn_mutex__t *txn_list_lock;
svn_mutex__t *fs_write_lock;
svn_mutex__t *txn_current_lock;
apr_pool_t *common_pool;
} fs_fs_shared_data_t;
Hotcopy needs to support ≥2 filesystem instances which have the same
contents and UUID. (Keeping the UUID is part of the API.) However,
everything above except common_pool needs to be per-instance. So, we
need some sort of per-instance identifier to use as the cache key.
So, for example, one way to proceed would be to maintain
a ${REPOS}/db/uuid2 file, which is not exposed to fs-loader.h, and which
is used as part of the key to ffd->ffsd structures.
Assuming we agree on the course, I should be able to get this
implemented this week.
Re: Tracebacks in svnadmin tests (fs->uuid issues)
Posted by Daniel Shahaf <da...@apache.org>.
AFAIK all of those are due to the same issue: dst_fs->uuid not being
initialized. Philip noted that there are probably worse problems here,
since fs_fs_shared_data_t (ffd->ffsd) holds various locks and caches tat
are indexed by UUID only.
This is at the top of my 'svn' todo list. (but that list is mostly
swapped out right now)
Thanks
Daniel
Greg Stein wrote on Fri, Apr 27, 2012 at 23:45:12 -0400:
> Hey Daniel,
>
> I had four svnadmin tests dump core due to an assertion: 8, 21, 26,
> 27. These are all marked XFail(), so my test run "passed".
>
> The stack traces on all four look like this:
>
> #4 0x000f4549 in svn_error__malfunction (can_return=1, file=0xa91bd
> "subversion/libsvn_fs_fs/fs.c", line=76, expr=0xa91b4 "fs->uuid") at
> subversion/libsvn_subr/error.c:698
> #5 0x00081455 in fs_serialized_init (fs=0x817f58,
> common_pool=0x80fa18, pool=0x80fa18) at
> subversion/libsvn_fs_fs/fs.c:76
> #6 0x00082040 in fs_hotcopy (src_fs=0x817f30, dst_fs=0x817f58,
> src_path=0x810028
> "/Users/gstein/src/svn/subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-8/db",
> dst_path=0x816a60
> "/Volumes/untitled/svn-test-work/repositories/svnadmin_tests-8.backup/db",
> clean_logs=0, incremental=0, cancel_func=0x240f <check_cancel>,
> cancel_baton=0x0, pool=0x80fa18) at subversion/libsvn_fs_fs/fs.c:321
> #7 0x00014337 in svn_fs_hotcopy2 (src_path=0x810028
> "/Users/gstein/src/svn/subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-8/db",
> dst_path=0x816a60
> "/Volumes/untitled/svn-test-work/repositories/svnadmin_tests-8.backup/db",
> clean=0, incremental=0, cancel_func=0x240f <check_cancel>,
> cancel_baton=0x0, scratch_pool=0x80fa18) at
> subversion/libsvn_fs/fs-loader.c:497
> #8 0x0005f8dd in svn_repos_hotcopy2 (src_path=0x80fc48
> "/Users/gstein/src/svn/subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-8",
> dst_path=0x80fce8 "", clean_logs=0, incremental=0, cancel_func=0x240f
> <check_cancel>, cancel_baton=0x0, pool=0x80fa18) at
> subversion/libsvn_repos/repos.c:1894
> #9 0x000052d7 in subcommand_hotcopy (os=0x80fb38, baton=0xbfffea7c,
> pool=0x80fa18) at subversion/svnadmin/main.c:1455
> #10 0x000075bf in main (argc=4, argv=0xbfffeb74) at
> subversion/svnadmin/main.c:2101
>
>
> The function calls are the same across the four. I have not examined
> them for line number or argument similarity, but just assumed all four
> had the same pattern.
>
> Cheers,
> -g