Re: Re: Flushing directory log accumulator during pool cleanup (Re: svn commit: r23342 - tr

"David Glasser" <glasserdavidglasser.net>
writes:

> Here's a simple patch that makes the test I added in
r27575 pass.  The
> whole test suite passes with this patch applied, but
since this bug
> involves error behavior, the test suite might not
actually cover it
> well.  I would like review before I commit.
>
> (An alternate implementation, which would still allow
*some* in-memory
> logs to be run during cleanup, would be to have a
boolean
> "logs_ok_to_run" flag on the directory baton,
which is cleared every
> time the accumulator is appended to and set only when
the accumulator
> is definitely runnable.)
>
> --dave
>
> [[[
> Fix wc corruption caused by flushing
potentially-incomplete logs
> during baton cleanup on error, by just not flushing
logs during baton
> cleanup.  Makes the new update test #42 pass.
>
> * subversion/libsvn_wc/update_editor.c
>   (cleanup_dir_baton): Don't call flush_log.
>
> * subversion/tests/cmdline/update_tests.py
>   (test_list): eof_in_interactive_conflict_resolver now
passes.
> ]]]

It's certainly a serious mistake to run incomplete log files
(unless
the implementation has had a major rework, I haven't really
been
following the development recently).

Your alternate implementation sounds much better: as I
understand it
your proposed patch means that if a large update is
interrupted there
could be lots of log "files" in memory
corresponding to megabytes of
downloaded data and effectively all this data would be
lost.

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On 11/5/07, Philip Martin <philipcodematters.co.uk>
wrote:
> "David Glasser" <glasserdavidglasser.net> writes:
>
> > Here's a simple patch that makes the test I added
in r27575 pass.  The
> > whole test suite passes with this patch applied,
but since this bug
> > involves error behavior, the test suite might not
actually cover it
> > well.  I would like review before I commit.
> >
> > (An alternate implementation, which would still
allow *some* in-memory
> > logs to be run during cleanup, would be to have a
boolean
> > "logs_ok_to_run" flag on the directory
baton, which is cleared every
> > time the accumulator is appended to and set only
when the accumulator
> > is definitely runnable.)
> >
> > --dave
> >
> > [[[
> > Fix wc corruption caused by flushing
potentially-incomplete logs
> > during baton cleanup on error, by just not
flushing logs during baton
> > cleanup.  Makes the new update test #42 pass.
> >
> > * subversion/libsvn_wc/update_editor.c
> >   (cleanup_dir_baton): Don't call flush_log.
> >
> > * subversion/tests/cmdline/update_tests.py
> >   (test_list):
eof_in_interactive_conflict_resolver now passes.
> > ]]]
>
> It's certainly a serious mistake to run incomplete log
files (unless
> the implementation has had a major rework, I haven't
really been
> following the development recently).
>
> Your alternate implementation sounds much better: as I
understand it
> your proposed patch means that if a large update is
interrupted there
> could be lots of log "files" in memory
corresponding to megabytes of
> downloaded data and effectively all this data would be
lost.

That's true.

On the other hand, if you're interrupting an operation, do
you really
want megabytes worth of work to be done before the process
finishes?

--dave


-- 
David Glasser | glasserdavidglasser.net | http://www.davidglasser.
net/

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org