Re: Tracebacks in svnadmin tests (fs->uuid issues)

Greg Stein wrote on Sun, Apr 29, 2012 at 03:32:51 -0400:
> On Apr 29, 2012 2:51 AM, "Daniel Shahaf" <danielsh_at_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.