On Thu, Aug 12, 2010 at 6:01 PM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Tue, Aug 3, 2010 at 12:40 PM, Paul Burba <ptburba_at_gmail.com> wrote:
>> Ok, I am waving the white flag as far as implementing a fix for issue
>> #3657 in the RA layer. I simply don't see how it can be done outside
>> of this: Put the DAV update report handler into send-all=TRUE
>> <S:update-report send-all=TRUE> mode when creating a
>> svn_ra_reporter3_t * during a merge/diff. This would prevent
>> <S:fetch-props/> response that causes the client to fetch *all* the
>> properties of a path which subsequently pushes these to the
>> svn_delta_editor_t callbacks as if they were all prop *changes* (as
>> Mike discussed here
>> http://subversion.tigris.org/issues/show_bug.cgi?id=3657#desc9).
>>
>> fyi: Currently this is how the two DAV RA layers use send-all mode:
>>
>> send-all send-all
>> mode mode
>> operation ra_serf ra_neon
>> --------- -------- ---------
>> update FALSE TRUE
>> status FALSE TRUE
>> switch FALSE TRUE
>> diff FALSE FALSE
>>
>> In that attached patch I reverted r966822 and tried the aforementioned
>> approach by tweaking ra_neon so the send-all mode on diff/merge is
>> TRUE. The whole test suite passes (including the test for issue
>> #3657). I haven't got an equivalent change to ra_serf to work yet,
>> but I'm not too worried about that yet because...
>>
>> ...After testing out this idea, I realized the problem with this
>> approach: It reopens issue #2048 'Primary connection (of parallel
>> network connections used) in ra-dav diff/merge timeout
>> unnecessarily.':
>>
>> http://subversion.tigris.org/issues/show_bug.cgi?id=2048
>> http://svn.apache.org/viewvc?view=revision&revision=853457
>>
>> So I'm at a dead-end right now. I'm happy to revert r966822 and
>> reopen issue #3657 if the consensus is that my initial fix is a sloppy
>> band-aid to cover incorrect ra_neon/ra_serf behavior and that the
>> latter two are where the fix belongs.
>>
>> Paul
>
> Mike suggested to me that the fix for this problem can be made in
> mod_dav_svn, since the update reporter already knows which properties
> have changed, *regardless* of the send-all mode, but it discards this
> information if send-all=FALSE and instead instead sends the client a
> 'fetch-props' response. As described previously in this thread, this
> causes the client to fetch all the properties of the path, not just
> the changes, and *all* the props are pushed through the editor
> callbacks as if they were all actual changes (which again is the core
> problem of issue #3657).
>
> I'm working on that approach,
I had pushed off the fix-it-in-mod_dav_svn approach till 1.8 since I
was going in circles with it, but recently found another real-world
example where this issue causes spurious conflicts on directory
properties, see
http://subversion.tigris.org/issues/show_bug.cgi?id=3657#desc13.
This issue has existed before 1.5 so it's nothing new, but given that
I found another non-theoretical example and given that the presence of
mergeinfo is only going to make this issue more common[1] I thought
I'd take another look for 1.7.
I still haven't gotten anywhere with the mod_dav_svn fix, so not much
to talk about there, but I did commit the fix for the directory
property conflicts in
http://svn.apache.org/viewvc?view=revision&revision=1056565. The fix
here is pretty much the same thing I did in
http://svn.apache.org/viewvc?view=revision&revision=966822: Filter out
the phony prop changes in the libsvn_client/repos_diff.c
svn_delta_editor_t change_[dir|file]_prop() callback implementations.
Just to be clear about one thing, when I committed r966822 Bert asked:
>> (change_file_prop): Only stash true property differences in the file
>> baton. The DAV RA providers may be sending us all the properties on
>> a file, not just the differences.
>
> Shouldn't this be fixed in the ra layer implementations then?
>
> This would be just 'guessing' if it is a change or a complete set.
Why not the RA layer? Two reasons. The first I explained earlier in
this thread, it reopens issue #2048 'Primary connection (of parallel
network connections used) in ra-dav diff/merge timeout unnecessarily.'
I won't go over that again, but even if there is a way to avoid that,
the RA layer is simply doing what we ask it when using the DAV
providers. Specifically, the mod_dav_svn update report handler, when
in 'skelta' mode[2] and describing changes to a path on which a
property has changed, asks the client to later fetch all properties
and figure out what has changed itself, i.e. the client asks the RA
layer to give us *all* the properties, it obliges, so where is the
problem as far as the RA layer is concerned?
So this ultimately needs to be fixed on the server. Given that, I am
leaving these libsvn_client filters in place so a 1.7 client will DTRT
with older servers (or maybe 1.7 servers too, since as I said, I'm not
having great success there).
As to the 'guessing', that was true. We'd go through the motions of
filtering over ra_local and ra_svn too. A pretty minor performance
hit, but in r1056565 I tweaked the filtering so it only applies when
using ra_serf and ra_neon.
Paul
[1] simply by increasing the potential pool of property changes
incoming during a merge.
[2] Which merges always are, regardless of whether we use ra_serf or ra_neon.
Paul
Received on 2011-01-08 01:09:27 CET