On Mon, Feb 13, 2012 at 12:05 PM, Daniel Shahaf <d...@daniel.shahaf.name> wrote:
> Branko Čibej wrote on Mon, Feb 13, 2012 at 17:52:01 +0100:
>> On 13.02.2012 17:09, Daniel Shahaf wrote:
>> > Thanks for your thoughts. One comment:
>> >
>> > Branko Čibej wrote on Sun, Feb 12, 2012 at 21:52:16 +0100:
>> >> The idea is that we'd always maintain the complete index, i.e., in order
>> >> to determine if path@15 exists, one only needs to search the index for
>> >> (path, rev <= 15, !deleted) -- which is trivial in a properly ordered
>> >> index. (Yes, this assumes that we record deletions in the index as well
>> >> as insertions.)
>> >>
>> >> All of this can be done with an append-only index representation. What
>> >> you can't do with append-only is represent forward history links, but --
>> >> we're not representing them very well right now, are we. :)
>> > Within the append-only constraint one can implement forward links if one
>> > has, say, a separate file per revision. (So asking "where was foo@15
>> > copied to" involves a linear scan of $REPOS/db/forward-links/15.)
>> >
>> > stsp and I even started on such a design on some branch somewhere ---
>> > but that branch has been abandoned as the motivation for it was rename
>> > tracking, which stsp figured he could solve better without the new FS
>> > feature.
>>
>> A separate file per revision has other problems, too. You'd quickly run
>> out of inodes on ext-derived file systems, for example.
>>
>
> Yeah, I was in theory mode in the previous mail. On the branch we
> didn't ignore those "implementation considerations" and went for 3 files
> per shard of 4k revisions.
>
> https://svn.apache.org/repos/asf/subversion/branches/fs-successor-ids/BRANCH-README
>
>> -- Brane
Hi All,
It seems we are far from any consensus on inheritable properties. The
major sticking points seem to be as follows:
-- 1 -- Performance: Server Side (a.k.a. "Is this ultimately an
exercise in futility?")
On Mon, Feb 6, 2012 at 6:41 PM, Greg Stein <gst...@gmail.com> wrote:
> In most data storage mechanisms for the repository, inheritable
> properties are a performance killer. Bill Tutt advised us against
> inheritable properties years ago for exactly this reason. It is one of
> the main reasons that we never implemented them.
...
> Consider any PATH -> DATA mapping for the storage. You now have to
> decompose PATH into N segments and issue N-1 more queries to look for
> inheritable properties.
Are there specific use-cases where performance is theoretically a
problem? Do you see the problem more on the repository side or the
client side (or both)?
Looking at the server side, I might be oversimplifying, but I don't
see how this is a performance killer (full disclosure: I've only
looked at fsfs so maybe bdb is a problem).
Let's say a client asks the server for some fictional inheritable
property svn:inheritable:foo on some subtree, e.g. 'svn pg
svn:inheritable:foo -v ^/subversion/branches/1.6.x/CHANGES'
The proposed svn_client_propget4 *only* supports getting inheritable
properties for TARGET, so the depth has no impact (why only supply the
inherited properties for TARGET? Because if DEPTH > EMPTY the client
can derive the inherited properties on the subtrees TARGET. Can't see
a need to make the server do this work):
svn_error_t *
svn_client_proplist4(const char *target,
const svn_opt_revision_t *peg_revision,
const svn_opt_revision_t *revision,
svn_depth_t depth,
const apr_array_header_t *changelists,
svn_boolean_t get_target_inherited_props,
svn_proplist_receiver2_t receiver,
void *receiver_baton,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
On the server the fsfs implementation for svn_fs_node_proplist,
fs_node_proplist, is eventually called for the target. It gets the
(possibly cached) dag_node_t for the target and then calls
svn_fs_fs__dag_get_proplist which gets the (also possibly cached)
node_revision_t and representation. Ok, nothing new yet. To find the
props TARGET inherits, we could have fs_node_proplist walk from the
target to the root of the repository repeating this process as it
looks for any inheritable properties. So certainly it's more work
than just getting the properties on the target, but it's hardly as
grim as a recursive propget on a large subtree.
If someone could spell out a use case where performance is likely to
be an issue, then that would go a long way to helping me understand
your concerns.
Brane and Stefan: I confess that you lost me with much of your
discussion re performance and the backend. Not surprising given that
most of my Subverion life has been spent in places other than
libsvn_fs. So I'm not ignoring you, I'm hoping that starting from a
use case I can maybe work my way to what you are saying.
-- 2 -- Can existing properties be made inheritable or are only
special new "inheritable" properties inhettiable?
In my original proposal inheritable properties are identified by a
prefix on the property name. This has the obvious drawback that
existing properties cannot be made inheritable. It has the advantage
of being simple though: It is immediately obvious to a user that a
given property is inheritable, it's right in the name. No need to
check another[1] property's value. Setting an inheritable property is
likewise a straightforward process.
So how do people feel? Must any inheritable property solution support
the ability to make existing props inheritable? Or is this not
essential?
[1] If we go down the road of allowing existing properties to be made
inheritable I don't see how we can avoid having to introduce a new
dedicated property that controls the inheritability of other versioned
properties. Imagine if today's sole inheritable prop, svn:mergeinfo,
didn't in-and-of-itself describe merges, but rather you had to
consider *another* property which dictated which children the
mergeinfo applied to. Seems cumbersome at best.
-- 3 -- Should only a subset of children inherit?
In my proposal, if you set an inheritable property on a repository
path@rev, then *all* of that path's children@rev inherit that
property. Stefan Fuhrmann suggested we consider a way to limit which
children inherit such properties. Personally I'm not in favor of
this, I don't think it gains us much, but we lose simplicity. I
constantly come back to my experience with svn:mergeinfo. The
inheritance model is very simple for mergeinfo (only paths without
mergeinfo can inherit it and then only from their nearest parent) and
yet many users have a tough time understanding it.
Agree? Disagree?
-- 4 -- Backwards Compatibility
On Mon, Feb 6, 2012 at 7:20 PM, Daniel Shahaf <danie...@elego.de> wrote:
> - How does all this interact with 1.7-and-older clients? For example,
> if svn:inheritable:foo is set on ^/trunk and a 1.7 client propgets
> this property on a descendant of ^/trunk that doesn't have that
> property set, I presume that client would be told that that property
> doesn't exist on the descendant?
On Tue, Feb 7, 2012 at 4:24 PM, Stefan Fuhrmann <eq...@web.de> wrote:
> I think the most problematic part of your proposal is backward
> compatibility as Daniel already pointed out. On a meta level,
> this may also be taken as a general weakness in your model
> that it does not interact well a proven, well-established model.
I already answered Daniel's question thusly: "Correct. You'll need a
1.8+ client and server to get the full benefit. Not much different
than merge tracking and svn:mergeinfo. If anybody has any suggestions
as to how a 1.7 client could suddenly recognize inheritable
properties, then I'm all ears!"
So Stefan, I'm all ears :-) I've re-read your alternative proposal
(http://svn.haxx.se/dev/archive-2012-02/0248.shtml) but don't see how
that helps a legacy client. Take for example any legacy client
subcommand that doesn't contact the repository, how can it possibly
know about any properties it inherits if said props are above the root
of the WC?
-- 5 -- Inheritance in the Working Copy
There is still much to be determined re how inheritance works within
the working copy (see Julian's notes in the wiki), but I'm leaving
this as an open issue until the preceeding items are addressed.
Thanks for all your feedback so far,
Paul
