On Wed, Dec 14, 2005 at 01:44:29PM -0500, Greg Hudson wrote:
> On Wed, 2005-12-14 at 11:14 +0000, Malcolm Rowe wrote:
> > However, doesn't this apply to the filesystem as well? Is there any
> > reason that the caller couldn't open the filesystem twice, create a
> > transaction through one svn_fs_t, and reopen it (by name) via another?
> > Wouldn't we need a global cache?
>
> There's no particular reason, no, but it seems unlikely that a caller
> would do that without a pool cleanup in between. (Perhaps my
> transaction argument is overblown for the same reason.
Perhaps. It seems more defensible to say that you should only open the
filesystem once than to say that you can't reopen the transaction.
> Part of my
> uneasiness here is that we seem to have a caller *somewhere* who is not
> aborting a transaction after receiving an error, and we don't understand
> where that caller is and how it works. I'm not sure the right approach
> is to work around this behavior in the FSFS code; perhaps the right
> answer is to document that you must give up on a transaction after
> receiving an error, and hunt down and eradicate any code which isn't
> doing so.)
That was the first thing I thought about (kill the transaction upon
error), but that's even more drastic than what I was suggesting, unless
by 'error' you meant something specific like 'an EIO error', rather
than possibly something like SVN_ERR_FS_NO_SUCH_ENTRY, a usage error.
Also, the commit code depends upon being able to retry a commit after
a SVN_ERR_FS_TXN_OUT_OF_DATE error, for example.
The other reasons why I haven't gone after the problem itself were:
* It looks like it might well be something to do with mod_dav_svn,
something I'm not set up to debug.
* If one client can do it, any client can - far better to prevent the
corruption in the first place.
* libsvn_fs_fs is responsible for the consistency of the FSFS filesystem,
not the caller.
So what I was thinking of was having a hash in svn_fs_t that mapped
transaction id => a per-transaction object; that object to hold the
file handles we need (as well as anything else we want uniquely
per-transaction); and reference-counted so that it's destroyed
automatically when the last 'handle' is closed. Does that sound sane?
[If the filesystem thing is still a worry, perhaps we could key a global
hash off the combination of repository GUID and transaction ID.]
One thing I was worrying about (possibly unnecessarily) was thread-safety.
There doesn't seem to be any real documentation about our policy (HACKING
doesn't mention it, for example), and I'm not sure what it is, other
than perhaps 'the libraries should be safe to use from multiple threads'.
For example, I could believe that it probably doesn't make sense to
allow two threads (or statically-linked processes, for that matter) to
simultaneously operate on the same transaction (partly because it'll
almost-certainly break, I think), but there's nothing I can rely on
for that.
(The reason I got to wondering about that was that I was trying to
work out how the entry-cache code -- which has three parallel arrays
to hold the id, entries hash, and pool -- could possibly be race-free,
given that there appears to be nothing that the code synchronises on to
read or write the cache).
But then again, it looks like the apr_hash_ functions aren't independently
synchronised either, so I'm guessing our thread-safety policy must be just
'you can call from any thread, but not at the same time'.
> > > If we're going
> > > to change the FS model, I think we want to look into getting rid of
> > > transactions as an object type, not intertwining them better with roots.
> > > (Although that would require revving some APIs.)
> Instead of creating or opening a txn, you'd create or open a txn root.
> There's no such thing as a "revision" object in the svn_fs interface, so
> I don't see the need for a "transaction" object either.
I agree (transactions would just be identified by name, yes?), but
it sounds like a lot of work for not much gain - or are there some
simplifications we could make if we cut out the svn_fs_txn_t type?
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Dec 27 01:33:01 2005