You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2009/03/24 13:20:30 UTC

Re: svn commit: r36749 - trunk/subversion/libsvn_subr

On Tue, Mar 24, 2009 at 06:03:48AM -0700, Bert Huijben wrote:
> Author: rhuijben
> Date: Tue Mar 24 06:03:48 2009
> New Revision: 36749
> 
> Log:
> * subversion/libsvn_subr/sqlite.c
>   (svn_sqlite__open): Switch sqlite synchronization off to make Sqlite
>     performance workable on systems implementing full synchronization.
>     Even with synchronization disabled Sqlite guarantees transactions over
>     application crashes above the WC.1.0 level; and yournaling filesystems
>     provide most of the other safety functions without this penalty.

What does this mean for systems that don't have journalling file systems?
Do they lose those other safety functions? What safety functions are lost
exactly?

What is "WC.1.0 level"? What thing can have or reach this level?
And is it something we can define or is it something defined internally
by sqlite?

(I don't know much about sqlite so please excuse my ignorance).

If this change can cause problems on platforms lacking features such
as journaling file systems, I'd rather make this a configurable setting
controlled by the user, with default set to safety, not performance.
At least in the stock svn client.

>     Added ### comment to help reviewing this decision before releasing 1.7.0.

> +                           /* ### Switch to normal when transactions
> +                             resolves the major performance penalty.*/

Can you make this comment more clear? The log message indicates the
comment was addressing some controversial issue, but the comment itself
is not very clear as to what this controversial issue is. (And it also
has a grammatical error.) Maybe I don't understand the comment because
I don't know the answers to my questions above?

Thanks,
Stefan

Re: svn commit: r36749 - trunk/subversion/libsvn_subr

Posted by Philip Martin <ph...@codematters.co.uk>.
Bert Huijben <rh...@sharpsvn.net> writes:

> Without disabling this synchronization a run of the the testsuite over
> fsfs-local will take a few days on Windows. With synchronization disabled I
> can get through it in about 40 minutes. 

I guess it works since you have tested it, but the documentation
http://www.sqlite.org/pragma.html doesn't mention the value NONE that
you are using, just the values FULL, NORMAL and OFF.  Is NONE the same
as OFF (perhaps because NONE gets converted to 0)?  Using OFF would be
better since that's a value that search engines find.

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1402564

Re: svn commit: r36749 - trunk/subversion/libsvn_subr

Posted by Mark Phippard <ma...@gmail.com>.
What about the usage of SQLite in fsfs?  How will it be impacted?  I'd
assume it'd be a lot easier to use transactions in fsfs, if it does
not already.

I ran the tests on OSX.  Here is the time for the WC 1.0 tests with
current trunk:

Summary of test results:
  1109 tests PASSED
  24 tests SKIPPED
  28 tests XFAILED (1 WORK-IN-PROGRESS)

real	23m57.808s
user	11m48.340s
sys	12m23.317s

And these are the results when export SVN_ENABLE_NG=y is done before
running the tests.

Summary of test results:
  1095 tests PASSED
  24 tests SKIPPED
  26 tests XFAILED (1 WORK-IN-PROGRESS)
  14 tests FAILED
  2 tests XPASSED
make: *** [check] Error 1

real	113m8.082s
user	31m29.505s
sys	26m52.110s

So I do not know how bad it was before your change, but it is still
pretty slow after it.

It is nice to see only 14 tests are failing.  Lots of progress has been made.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1404253

Re: svn commit: r36749 - trunk/subversion/libsvn_subr

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Mar 24, 2009 at 02:55:49PM +0100, Bert Huijben wrote:
> > -----Original Message-----
> > From: Stefan Sperling [mailto:stsp@elego.de]
> > Sent: dinsdag 24 maart 2009 14:21
> > To: dev@subversion.tigris.org
> > Subject: Re: svn commit: r36749 - trunk/subversion/libsvn_subr
> > 
> > On Tue, Mar 24, 2009 at 06:03:48AM -0700, Bert Huijben wrote:
> > > Author: rhuijben
> > > Date: Tue Mar 24 06:03:48 2009
> > > New Revision: 36749
> > >
> > > Log:
> > > * subversion/libsvn_subr/sqlite.c
> > >   (svn_sqlite__open): Switch sqlite synchronization off to make Sqlite
> > >     performance workable on systems implementing full synchronization.
> > >     Even with synchronization disabled Sqlite guarantees transactions
> over
> > >     application crashes above the WC.1.0 level; and yournaling
> filesystems
> > >     provide most of the other safety functions without this penalty.
> > 
> > What does this mean for systems that don't have journalling file systems?
> > Do they lose those other safety functions? What safety functions are lost
> > exactly?
> > 
> > What is "WC.1.0 level"? What thing can have or reach this level?
> > And is it something we can define or is it something defined internally
> > by sqlite?
> > 
> > (I don't know much about sqlite so please excuse my ignorance).
> 
> Currently Sqlite asks the OS after every partial write to wait for flushing
> the buffers on disk. This is implemented differently on diverse operating
> systems. On most linux filesystems this is +- a NO-OP (Filesystem determines
> it is not necessary because it has journaling support). 
> On Windows this is exactly what it says and on MAC-OS it is somewhere in
> between unless you enable another sync pragma that is by default disabled.
> 
> Without this safety net, that is designed to protect disk access errors in
> case of power failures and hardware breakage Sqlite is documented to be MUCH
> (Upto 50 times) faster, but in our current usage the difference is even
> bigger. (+- every record change is currently a transaction by itself).
> 
> 
> Over this OS synchronization layer Sqlite has 'normal' transaction support
> with atomic commits on about the same safety level as Mysql. This layer,
> without the safety net just disabled guarantees that the transactions are
> atomic over application crashes and virtually all other malfunctions. 
> 
> The rest of our datastore (pristine files, property access, normal file
> change detection) currently comes near these guarantees in this lowest
> security mode; so all layers above are just overkill.

OK, so with sqlite synchronization enabled we effectively disable disk
caches on some systems. In this light I am OK with your change.
We don't need to be safer than wc-1.0 already is.

Thanks for the explanation!
Maybe put a bit of this information in the comment?

Stefan

RE: svn commit: r36749 - trunk/subversion/libsvn_subr

Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@elego.de]
> Sent: dinsdag 24 maart 2009 14:21
> To: dev@subversion.tigris.org
> Subject: Re: svn commit: r36749 - trunk/subversion/libsvn_subr
> 
> On Tue, Mar 24, 2009 at 06:03:48AM -0700, Bert Huijben wrote:
> > Author: rhuijben
> > Date: Tue Mar 24 06:03:48 2009
> > New Revision: 36749
> >
> > Log:
> > * subversion/libsvn_subr/sqlite.c
> >   (svn_sqlite__open): Switch sqlite synchronization off to make Sqlite
> >     performance workable on systems implementing full synchronization.
> >     Even with synchronization disabled Sqlite guarantees transactions
over
> >     application crashes above the WC.1.0 level; and yournaling
filesystems
> >     provide most of the other safety functions without this penalty.
> 
> What does this mean for systems that don't have journalling file systems?
> Do they lose those other safety functions? What safety functions are lost
> exactly?
> 
> What is "WC.1.0 level"? What thing can have or reach this level?
> And is it something we can define or is it something defined internally
> by sqlite?
> 
> (I don't know much about sqlite so please excuse my ignorance).

Currently Sqlite asks the OS after every partial write to wait for flushing
the buffers on disk. This is implemented differently on diverse operating
systems. On most linux filesystems this is +- a NO-OP (Filesystem determines
it is not necessary because it has journaling support). 
On Windows this is exactly what it says and on MAC-OS it is somewhere in
between unless you enable another sync pragma that is by default disabled.

Without this safety net, that is designed to protect disk access errors in
case of power failures and hardware breakage Sqlite is documented to be MUCH
(Upto 50 times) faster, but in our current usage the difference is even
bigger. (+- every record change is currently a transaction by itself).


Over this OS synchronization layer Sqlite has 'normal' transaction support
with atomic commits on about the same safety level as Mysql. This layer,
without the safety net just disabled guarantees that the transactions are
atomic over application crashes and virtually all other malfunctions. 

The rest of our datastore (pristine files, property access, normal file
change detection) currently comes near these guarantees in this lowest
security mode; so all layers above are just overkill.


This forced flushing behavior made a checkout of the greek tree with WC-NG
take about a minute on my pc. With WC-1.0 this was < 1 second and with WC-NG
with synchronization disabled about 1.5-2.0 seconds.


Without disabling this synchronization a run of the the testsuite over
fsfs-local will take a few days on Windows. With synchronization disabled I
can get through it in about 40 minutes. 

(Greg reported about an hour running time on his non-Windows system last
Friday before I experimented with disabling syncronization).

	Bert
> 
> If this change can cause problems on platforms lacking features such
> as journaling file systems, I'd rather make this a configurable setting
> controlled by the user, with default set to safety, not performance.
> At least in the stock svn client.
> 
> >     Added ### comment to help reviewing this decision before releasing
> 1.7.0.
> 
> > +                           /* ### Switch to normal when transactions
> > +                             resolves the major performance penalty.*/
> 
> Can you make this comment more clear? The log message indicates the
> comment was addressing some controversial issue, but the comment itself
> is not very clear as to what this controversial issue is. (And it also
> has a grammatical error.) Maybe I don't understand the comment because
> I don't know the answers to my questions above?
> 
> Thanks,
> Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1402337