Re: Tree-conflicts: HEADS UP: update failure with tc-siblings :0

Quoting "Neels J. Hofmeyr" <neels_at_elego.de>:

>
>
> Neels J. Hofmeyr wrote:
>> Stephen Butler wrote:
>>>> CMD: svn status -v -u -q ...
>>>> D 2 1 jrandom /A/B/E/beta
>>>> D C * 1 1 jrandom /A/B/E/alpha
>>>> D C 2 2 jrandom /A/B/E
>>>> D * 1 1 jrandom /A/B/lambda
>>>> 2 1 jrandom /A/B/F
>>> The missing ' C' for lambda is still missing on trunk. It appears
>>> that two log files are written in /A/B/.svn, for lambda and for E, and
>>> svn_wc__run_log() reads both of them, but the content of the first log
>>> file isn't written to the entry.
>>>
>>> Strange. I'll keep digging.
>
> I've also done some digging now and am busy fixing it. (Steve, I know you
> are or have been on this. Shout if I'm duplicating your work or something.)

Actually, I kept working on the skipping-tree-conflict-victims issue.

> Let's explain the problem:
>
> run_log() reads two modify-entry commands from a log file:
>
> <modify-entry
> name=""
> tree-conflicts="B:dir:update:edited:deleted"/>
> <modify-entry
> name=""
> tree-conflicts="mu:file:update:edited:deleted"/>
>
> It calls log_do_modify_entry() on both of them.
>
> Common sense says that this tree-conflicts data should be concatenated.
> But log_do_modify_entry() *replaces* the information that is already there.
> So the first new conflict is overwritten by the second.
>
> Ok, let's step one layer out of this onion. There is the function
> svn_wc__loggy_add_tree_conflict() which writes these tree-conflicts to the
> log. This function first checks what other tree-conflicts there are in the
> entry, adds the new tree-conflicts data and replaces the whole
> tree-conflicts data in the entry with the new list.
>
> The problem is that if in one directory, two tree-conflicts appear in the
> same run and are recorded using the same log, the second tree-conflict
> addition again reads the *old* list of conflicts from the entry, appends the
> second entry and writes the whole thing out. So, any earlier tree-conflicts
> that were queued in the log file and aren't in the official entry yet get
> overwritten with the latest one.
>
> I am busy on a patch that first collects all new tree-conflicts upon update
> in the parent dir baton as svn_wc_conflict_description_t* in an apr_array
> and logs them all at once on close_directory() (in update_editor.c).
>
> The patch won't make it today, but there are questions I'd like to send out
> already:
>
> 1)
> In libsvn_wc/update_editor.c, around line 1632 in
> do_entry_deletion(), it says:
>
> [[[
> if (tree_conflict != NULL)
> {
> /* Run the log immediately, so that the tree conflict is recorded. */
> SVN_ERR(svn_wc__write_log(adm_access, *log_number, log_item, pool));
> SVN_ERR(svn_wc__run_log(adm_access, NULL, pool));
> *log_number = 0;
> }
> ]]]
>
> Can anyone remember why that would be necessary?

Because we write the tree conflict data to a local stringbuf (log_item).

I think that's a good thing, because if we save up lots of tree conflict
data until the parent dir closes, and the user cancels, there could be
some tree conflicts lost.

>
>
> 2)
> We use svn_wc_conflict_description_create_tree() to make a new conflict
> description struct. The fact that this function also *allocates* the memory
> makes it a little harder to push a new description onto an
> apr_array_header_t, since apr_array_push() is also allocating memory and
> returning it to be written in.
>
> I'm currently solving this by not storing conflict description structs, but
> only pointers to them in the apr_array. I thus avoid changing the signature
> of svn_wc_conflict_description_create_tree(), which would otherwise break
> with the conventions of svn_wc_conflict_description_create_text() and
> svn_wc_conflict_description_create_prop(). Do you guys agree with that, or
> would you rather change the function signature or even signatureS?

Instead of keeping conflict structs in memory until the parent dir is
closed, why don't we write each tree conflict immediately? That would
eliminate the possibility of lost conflicts on cancel.

Specifically, use a local stringbuf instead of db->log_accum, and
write & run the log immediately as in do_entry_deletion. That would
add a few disk accesses, but the code does a lot of those already. I
don't think we gain much performance by adding TCs in bunches.

Steve

-- 
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE16321419
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org