Re: Race in svn_atomic_namespace__create
On Wed, Oct 31, 2012 at 03:24:10PM +0100, Stefan Fuhrmann wrote: On Wed, Oct 31, 2012 at 2:54 PM, Philip Martin philip.mar...@wandisco.comwrote: Philip Martin philip.mar...@wandisco.com writes: Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes: Excellent analysis, Philip! With r1404112, we use plain APR mmap code with almost no coding overhead. The only downside is that we now have a temporary file sitting in the db folder. Error handling needs attention: $ svnadmin create repo $ svnadmin dump repo /dev/null $ chmod -rw repo/db/rev-prop-atomicsShm $ svnadmin dump repo /dev/null Segmentation fault We are mmaping a 64k file, that's bigger than a disk block on lots of filesystems so updates are not atomic. Do we have to consider corruption: $ svnadmin create repo $ dd if=/dev/urandom of=repo/db/rev-prop-atomicsShm bs=64k count=1 $ svnadmin verify repo Segmentation fault $ svnadmin recover repo Repository lock acquired. Please wait; recovering the repository may take some time... Recovery completed. The latest repos revision is 0. $ svnadmin verify repo Segmentation fault Perhaps recover should delete the file? Done. Also, random data should no longer result in segfaults. I just came across something that reminded me of this thread. It seems PostgreSQL is doing something quite similar to what we want to do here: When the first PostgreSQL process attaches to the shared memory segment, it checks how many processes are attached. If the result is anything other than one, it knows that there's another copy of PostgreSQL running which is pointed at the same data directory, and it bails out. http://rhaas.blogspot.nl/2012/06/absurd-shared-memory-limits.html If this works for postgres I wonder why it wouldn't work for us. Is this something we cannot do because APR doesn't provide the necessary abstractions?
Re: svn commit: r1405517 - in /subversion/trunk/subversion/bindings/javahl/native: CopySources.cpp CreateJ.cpp SVNClient.cpp StringArray.cpp Targets.cpp
On Nov 4, 2012, at 2:14 AM, stef...@apache.org wrote: Author: stefan2 Date: Sun Nov 4 10:14:56 2012 New Revision: 1405517 URL: http://svn.apache.org/viewvc?rev=1405517view=rev Log: Silence integer size conversion warnings in JavaHL under Win64 by casting the values explicitly. We assume that argument counts and property sizes are all well below the 2G limit. Hi Stefan, Since you're in C++ here you could switch to using static_cast or reinterpret_cast instead, that would be better for self documentation, although that doesn't seem to be the style in the code. Blair
Re: svn commit: r1405517 - in /subversion/trunk/subversion/bindings/javahl/native: CopySources.cpp CreateJ.cpp SVNClient.cpp StringArray.cpp Targets.cpp
On 04.11.2012 12:37, Blair Zajac wrote: On Nov 4, 2012, at 2:14 AM, stef...@apache.org wrote: Author: stefan2 Date: Sun Nov 4 10:14:56 2012 New Revision: 1405517 URL: http://svn.apache.org/viewvc?rev=1405517view=rev Log: Silence integer size conversion warnings in JavaHL under Win64 by casting the values explicitly. We assume that argument counts and property sizes are all well below the 2G limit. Hi Stefan, Since you're in C++ here you could switch to using static_cast or reinterpret_cast instead, that would be better for self documentation, although that doesn't seem to be the style in the code. static_cast. *not* reinterpret_cast, which we should never need in high-level code. -- Brane
Re: svn commit: r1400498 - /subversion/trunk/subversion/svnadmin/main.c
On Sun, Oct 21, 2012 at 9:42 PM, Daniel Shahaf d...@daniel.shahaf.namewrote: stef...@apache.org wrote on Sat, Oct 20, 2012 at 18:35:24 -: Author: stefan2 Date: Sat Oct 20 18:35:24 2012 New Revision: 1400498 URL: http://svn.apache.org/viewvc?rev=1400498view=rev Log: On systems without efficient 64 bit atomics, svnadmin should not attempt to enable revprop caching because FSFS will reject it and log a warning. svnadmin writes the latter to stderr - which confuses our tests. * subversion/svnadmin/main.c (open_repos): enable revprop caching only if efficient Modified: subversion/trunk/subversion/svnadmin/main.c Modified: subversion/trunk/subversion/svnadmin/main.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/main.c?rev=1400498r1=1400497r2=1400498view=diff == --- subversion/trunk/subversion/svnadmin/main.c (original) +++ subversion/trunk/subversion/svnadmin/main.c Sat Oct 20 18:35:24 2012 @@ -43,6 +43,7 @@ #include svn_xml.h #include private/svn_opt_private.h +#include private/svn_named_atomic.h #include svn_private_config.h @@ -115,7 +116,8 @@ open_repos(svn_repos_t **repos, apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS, APR_HASH_KEY_STRING, 1); apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS, - APR_HASH_KEY_STRING, 1); + APR_HASH_KEY_STRING, + svn_named_atomic__is_efficient() ? 1 : 0); svnadmin shouldn't use private APIs here. (Which probably means this code is a layering violation) Maybe add a value that means enable if it would be efficient? Or just unconditionally never enable the cache (regardless of the config) when it wouldn't be efficient. r1405553 implements something along those lines now. -- Stefan^2. -- Certified Supported Apache Subversion Downloads: * http://www.wandisco.com/subversion/download *
Re: svn commit: r1405539 - /subversion/trunk/subversion/bindings/javahl/native/
On 11/04/2012 04:48 AM, stef...@apache.org wrote: Author: stefan2 Date: Sun Nov 4 12:47:59 2012 New Revision: 1405539 URL: http://svn.apache.org/viewvc?rev=1405539view=rev Log: Make all explicit casts, i.e. those not inside some APR macro, use the C++ cast operators static_cast, const_cast and reinterpret_cast. Cool, thanks! Blair
Re: Authz on Collection of Repositories
Thanks Ivan for your work. I have very little experience with the svn codebase so my review is probably not very valuable. Anyway. looks good to me. I have meant to set up a test server with our reference configuration to validate the patch under realistic circumstances. Unfortunately, the SLES activation servers have been down for several hours (we don't have dev tools on our VM Appliance by default). I will do some tests with parentpath under /svn/ and both variations of Satisfy as soon as possible. On 2 nov 2012, at 15:25, C. Michael Pilato wrote: I think HEAD[1] request would be the appropriate request here. (And I wonder, in retrospect, why we aren't using it for our regular in-repos path-based authz...) I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below). The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning 200 OK. /Thomas Å.
[svnbench] Revision: 1405681 compiled Nov 5 2012, 00:21:40 on x86_64-unknown-linux-gnu
1.7.0@1181106 vs. trunk@1405553 Started at Mon Nov 5 00:25:05 UTC 2012 *DISCLAIMER* - This tests only file://-URL access on a GNU/Linux VM. This is intended to measure changes in performance of the local working copy layer, *only*. These results are *not* generally true for everyone. Charts of this data are available at http://svn-qavm.apache.org/charts/ Averaged-total results across all runs: --- Compare trunk@1405553 to 1.7.0 Navg operation 56/90.68|-50.010 TOTAL RUN 3K/5300.77| -0.005 add 112/180.55| -0.446 checkout 448/720.86| -1.365 commit 56/91.19| +0.051 copy 56/90.76| -0.081 delete 280/450.12| -5.102 info 112/180.66| -1.306 merge 2K/5160.92| -0.001 mkdir 152/210.67| -0.004 propdel 42K/6K0.74| -0.003 proplist 43K/6K0.75| -0.003 propset 3K/5910.71| -0.003 ps 112/180.88| -0.001 resolve 112/180.67| -0.076 resolved 784/1260.68| -0.065 status 56/90.80| -0.284 switch 784/1260.65| -0.294 update (legend: 1.23|+0.45 means: slower by factor 1.23 and by 0.45 seconds; factor 1 and seconds 0 means 'trunk@1405553' is faster. 2/3 means: '1.7.0' has 2 timings on record, the other has 3.) Above totals split into separate dir-levelsxdir-spread runs: Compare trunk@1405553,5x5 to 1.7.0,5x5 Navg operation 19/30.68|-134.944 TOTAL RUN 2K/4560.75| -0.006 add 38/60.55| -1.187 checkout 152/240.88| -3.210 commit 19/31.42| +0.116 copy 19/30.76| -0.199 delete 95/150.11|-14.858 info 38/60.68| -3.399 merge 2K/4700.89| -0.002 mkdir 152/200.65| -0.004 propdel 39K/6K0.75| -0.003 proplist 40K/6K0.75| -0.003 propset 3K/5520.71| -0.004 ps 38/60.84| -0.002 resolve 38/60.64| -0.220 resolved 266/420.66| -0.177 status 19/30.84| -0.598 switch 266/420.65| -0.742 update (legend: 1.23|+0.45 means: slower by factor 1.23 and by 0.45 seconds; factor 1 and seconds 0 means 'trunk@1405553,5x5' is faster. 2/3 means: '1.7.0,5x5' has 2 timings on record, the other has 3.) Compare trunk@1405553,100x1 to 1.7.0,100x1 Navg operation 19/30.78| -6.204 TOTAL RUN 532/710.96| -0.001 add 38/60.61| -0.075 checkout 152/240.83| -0.336 commit 19/30.91| -0.026 copy 19/30.79| -0.023 delete 95/150.48| -0.137 info 38/60.64| -0.228 merge 266/461.24| +0.004 mkdir 2K/3370.72| -0.003 proplist 1K/2730.76| -0.003 propset 133/330.75| -0.003 ps 38/60.86| -0.001 resolve 38/61.10| +0.007 resolved 266/420.93| -0.005 status 19/30.58| -0.160 switch 266/420.65| -0.095 update (legend: 1.23|+0.45 means: slower by factor 1.23 and by 0.45 seconds; factor 1 and seconds 0 means 'trunk@1405553,100x1' is faster. 2/3 means: '1.7.0,100x1' has 2 timings on record, the other has 3.) Compare trunk@1405553,1x100 to 1.7.0,1x100 Navg operation 18/30.86| -1.212 TOTAL RUN 18/30.57| -0.019 add 36/60.63| -0.028 checkout 144/240.90| -0.073 commit 18/31.27| +0.063 copy 18/30.77| -0.006 delete 90/150.85| -0.008 info 36/60.53| -0.100 merge 666/1110.75| -0.002 proplist 756/1260.75| -0.003 propset 36/60.74| -0.003 ps 36/60.93| -0.001 resolve 36/60.71| -0.005 resolved 252/420.75| -0.004 status 18/30.67| -0.024 switch 252/420.89| -0.005 update (legend: 1.23|+0.45 means: slower by factor 1.23 and by 0.45 seconds; factor 1 and seconds 0 means 'trunk@1405553,1x100' is faster. 2/3 means: '1.7.0,1x100' has 2 timings on record, the other has 3.) More detail: Timings for 1.7.0,5x5 Nmin max avg operation (unit is seconds) 19 391.94 561.33 419.35 TOTAL RUN 2K0.012.290.02 add 380.025.812.65 checkout 1521.53 160.96 26.21 commit 190.190.490.27 copy 190.741.240.85 delete 958.63 43.37 16.73 info 386.40 21.02 10.54 merge 2K0.010.660.01 mkdir 1520.010.090.01 propdel 39K0.010.690.01 proplist 40K0.011.100.01 propset 3K0.010.700.01 ps 380.010.010.01 resolve 380.450.960.60 resolved 2660.202.360.52 status 193.134.683.68 switch 2660.257.022.14 update -- Timings for trunk@1405553,5x5 Nmin max avg operation (unit is seconds) 3 282.39 286.24 284.41 TOTAL RUN
Hanging tests mergeinfo-test.exe 12, 15 and skel-test.exe 4 on WinXP with VS2010 release build
Before I look into this further, I'd like to know if anyone else has seen this or can reproduce this (or doesn't reproduce this with a similar environment) ... Tests 12 and 15 of mergeinfo-test.exe and test 4 of skel-test.exe hang on my machine (i.e. they don't do anything useful anymore, but do fully consume a cpu core until I kill them). This is with trunk@1405553. All other tests, except the above three, run successfully for me. I'm running Windows XP (32 bit). I only see this happening with a release build (shared; haven't tried with --disable-shared yet), when compiled with VS 2010. I do *not* see this (i.e. tests complete normally) with: - debug build made with VS 2010. - release or debug builds made with VSExpress 2008. Does anyone else have a release build on (any) Windows, made with VS 2010, and can run these tests? I'm using these dependencies: Httpd 2.2.22 Apr 1.4.5 Apr-Util 1.4.1 Apr-Iconv 1.2.1 OpenSSL 1.0.1c Serf 1.1.1 SQLite 3.7.14.1 ZLib 1.2.6 -- Johan
