You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Collins-Sussman <su...@red-bean.com> on 2006/01/10 17:51:22 UTC
The use of trails in svn_fs_base__commit_txn()
After chatting with kfogel, I have two concerns regarding our use of
trails in svn_fs_base__commit_txn(). We agreed that they were worth
mailing the list about.
1. The first time a trail is used in the commit process, it's to
fetch the root node of specific revision "close" (or equal) to the
youngest rev. The comment doesn't make any sense; it seems to
imply that the trail is somehow protecting us from the 'root'
changing, though it's not at all clear what it means. The trail
seems totally unnecessary to me, and indeed, FSFS just goes and
getches the root-node directly. Can we remove this trail?
Toss the comment?
2. The second time a trail is used in the commit process, it's to
isolate the merge() operation (whereby the transacion tree is
merged into the latest revision.) I'm assuming that fs_base does
this work in a trail so that if a CONFLICT error occurs, the
transaction tree merges can be undone. In other words, I suspect
fs_base returns the tree exactly as it found it. FSFS, on the
other hand, makes no such guarantee. If its merging attempt
fails, the transaction tree may be left full of edits.
I'm not sure what our guarantees to callers are. Is FSFS not
being nice enough, or is fs_base being overly nice?
Either way, it seems to not matter in practice. I'm guessing that
every caller of commit_txn() simply aborts the transaction if an
error is returned, so we've never noticed. Thoughts, concerns?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The use of trails in svn_fs_base__commit_txn()
Posted by Michael Pilato <cm...@collab.net>.
On Tue, 2006-01-10 at 12:25 -0600, kfogel@collab.net wrote:
> > 2. The second time a trail is used in the commit process, it's to
> > isolate the merge() operation (whereby the transacion tree is
> > merged into the latest revision.) I'm assuming that fs_base does
> > this work in a trail so that if a CONFLICT error occurs, the
> > transaction tree merges can be undone. In other words, I suspect
> > fs_base returns the tree exactly as it found it. FSFS, on the
> > other hand, makes no such guarantee. If its merging attempt
> > fails, the transaction tree may be left full of edits.
> >
> > I'm not sure what our guarantees to callers are. Is FSFS not
> > being nice enough, or is fs_base being overly nice?
> >
> > Either way, it seems to not matter in practice. I'm guessing that
> > every caller of commit_txn() simply aborts the transaction if an
> > error is returned, so we've never noticed. Thoughts, concerns?
>
> Yeah, I don't have a feel for which is the "better" behavior here, but
> it sure would be nice if we were consistent. Our callers will no
> doubt change someday, and then we'd be in for a surprise...
That last-minute changes which snuck in during my transaction's commit
are merged into my transaction at commit-time is an implementation
detail, and certainly not part of the expected behavior as viewed at an
API level. In other words, looking strictly at the svn_fs.h API,
there's nothing there that would make me *not* be completely surprised
to find that my failed-due-to-conflicts transaction was modified by the
commit process. BDB is doing the Right thing here; FSFS is not.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: The use of trails in svn_fs_base__commit_txn()
Posted by kf...@collab.net.
Ben Collins-Sussman <su...@red-bean.com> writes:
> My feeling is: let's just augment svn_fs_commit_txn()'s docstring to
> say something like, "if an error is returned, the transaction is in an
> indeterminate state, consider it useless." Our callers are already
> aborting the txn anyway, but it would be nice to make this policy
> 'official'.
+1.
If we want to change this in the future, we can, since it would be
backwards-compatible to do so. We might consistify the code between
fsfs and bdb, and then have all txns in some well-defined state
depending on the error returned. We can update the documentation
if/when we make that change.
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The use of trails in svn_fs_base__commit_txn()
Posted by Julian Foad <ju...@btopenworld.com>.
Greg Hudson wrote:
> On Tue, 2006-01-10 at 16:46 -0600, Ben Collins-Sussman wrote:
>
>>Yeah, rollback is 'hard' without real db transactions. Let's go shopping. :-)
>
> My philosophy with FSFS was that Subversion transactions *are* the FSFS
> equivalent of DB transactions. Giving them ACID properties is
> redundant, since they are the mechanism by which the FSFS repository
> itself gets ACID properties.
>
> Of course, if you want to use Subversion FS transactions for some other
> reason than ACID commits to the repository itself, the FSFS
> implementation comes up short. You can't modify the same transaction
> from two threads or processes at the same time, and you can't care too
> much about their fate since a failed operation generally destroys them.
It sounds like it would be useful to put those notes in the source
documentation if you can find a good place.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The use of trails in svn_fs_base__commit_txn()
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2006-01-10 at 16:46 -0600, Ben Collins-Sussman wrote:
> Yeah, rollback is 'hard' without real db transactions. Let's go shopping. :-)
My philosophy with FSFS was that Subversion transactions *are* the FSFS
equivalent of DB transactions. Giving them ACID properties is
redundant, since they are the mechanism by which the FSFS repository
itself gets ACID properties.
Of course, if you want to use Subversion FS transactions for some other
reason than ACID commits to the repository itself, the FSFS
implementation comes up short. You can't modify the same transaction
from two threads or processes at the same time, and you can't care too
much about their fate since a failed operation generally destroys them.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The use of trails in svn_fs_base__commit_txn()
Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 1/10/06, Michael Pilato <cm...@collab.net> wrote:
> On Tue, 2006-01-10 at 14:36 -0600, Ben Collins-Sussman wrote:
>
> > My feeling is: let's just augment svn_fs_commit_txn()'s docstring to
> > say something like, "if an error is returned, the transaction is in an
> > indeterminate state, consider it useless." Our callers are already
> > aborting the txn anyway, but it would be nice to make this policy
> > 'official'.
>
> Translation: There's no good reason to trash the txn, but it sure is
> hard to make FSFS to the sensible thing here, so let's punt!
Yeah, rollback is 'hard' without real db transactions. Let's go shopping. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The use of trails in svn_fs_base__commit_txn()
Posted by Michael Pilato <cm...@collab.net>.
On Tue, 2006-01-10 at 14:36 -0600, Ben Collins-Sussman wrote:
> My feeling is: let's just augment svn_fs_commit_txn()'s docstring to
> say something like, "if an error is returned, the transaction is in an
> indeterminate state, consider it useless." Our callers are already
> aborting the txn anyway, but it would be nice to make this policy
> 'official'.
Translation: There's no good reason to trash the txn, but it sure is
hard to make FSFS to the sensible thing here, so let's punt!
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: The use of trails in svn_fs_base__commit_txn()
Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 10 Jan 2006 12:25:59 -0600, kfogel@collab.net <kf...@collab.net> wrote:
> Ben Collins-Sussman <su...@red-bean.com> writes:
> > 1. The first time a trail is used in the commit process, it's to
> > fetch the root node of specific revision "close" (or equal) to the
> > youngest rev. The comment doesn't make any sense; it seems to
> > imply that the trail is somehow protecting us from the 'root'
> > changing, though it's not at all clear what it means. The trail
> > seems totally unnecessary to me, and indeed, FSFS just goes and
> > getches the root-node directly. Can we remove this trail?
> > Toss the comment?
>
> Yes, my feeling is fetch the node in a non-traily manner, and edit or
> toss the comment as appropriate.
Maybe I'll send a patch, then.
>
> > 2. The second time a trail is used in the commit process, it's to
> > isolate the merge() operation (whereby the transacion tree is
> > merged into the latest revision.) I'm assuming that fs_base does
> > this work in a trail so that if a CONFLICT error occurs, the
> > transaction tree merges can be undone. In other words, I suspect
> > fs_base returns the tree exactly as it found it. FSFS, on the
> > other hand, makes no such guarantee. If its merging attempt
> > fails, the transaction tree may be left full of edits.
> >
> > I'm not sure what our guarantees to callers are. Is FSFS not
> > being nice enough, or is fs_base being overly nice?
> >
> > Either way, it seems to not matter in practice. I'm guessing that
> > every caller of commit_txn() simply aborts the transaction if an
> > error is returned, so we've never noticed. Thoughts, concerns?
>
> Yeah, I don't have a feel for which is the "better" behavior here, but
> it sure would be nice if we were consistent. Our callers will no
> doubt change someday, and then we'd be in for a surprise...
>
My feeling is: let's just augment svn_fs_commit_txn()'s docstring to
say something like, "if an error is returned, the transaction is in an
indeterminate state, consider it useless." Our callers are already
aborting the txn anyway, but it would be nice to make this policy
'official'.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The use of trails in svn_fs_base__commit_txn()
Posted by kf...@collab.net.
Ben Collins-Sussman <su...@red-bean.com> writes:
> 1. The first time a trail is used in the commit process, it's to
> fetch the root node of specific revision "close" (or equal) to the
> youngest rev. The comment doesn't make any sense; it seems to
> imply that the trail is somehow protecting us from the 'root'
> changing, though it's not at all clear what it means. The trail
> seems totally unnecessary to me, and indeed, FSFS just goes and
> getches the root-node directly. Can we remove this trail?
> Toss the comment?
Yes, my feeling is fetch the node in a non-traily manner, and edit or
toss the comment as appropriate.
> 2. The second time a trail is used in the commit process, it's to
> isolate the merge() operation (whereby the transacion tree is
> merged into the latest revision.) I'm assuming that fs_base does
> this work in a trail so that if a CONFLICT error occurs, the
> transaction tree merges can be undone. In other words, I suspect
> fs_base returns the tree exactly as it found it. FSFS, on the
> other hand, makes no such guarantee. If its merging attempt
> fails, the transaction tree may be left full of edits.
>
> I'm not sure what our guarantees to callers are. Is FSFS not
> being nice enough, or is fs_base being overly nice?
>
> Either way, it seems to not matter in practice. I'm guessing that
> every caller of commit_txn() simply aborts the transaction if an
> error is returned, so we've never noticed. Thoughts, concerns?
Yeah, I don't have a feel for which is the "better" behavior here, but
it sure would be nice if we were consistent. Our callers will no
doubt change someday, and then we'd be in for a surprise...
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org