Re: svn commit: r990916 - in /subversion/trunk/subversion/bindings/javahl/native: ClientContext.cpp ClientContext.h
Bert Huijben b...@qqmail.nl writes: (I see a completely different test failure on the Windows bot) On Linux I get the same as Hyrum. Can you run this same test with single-db? In single-db on Linux I get 3 Failures rather than 1 Error: .F..F..F . Time: 139.136 There were 3 failures: 1) testBasicRevert(org.tigris.subversion.javahl.BasicTests)junit.framework.AssertionFailedError: status not found in working copy: A/B/E/alpha at org.tigris.subversion.javahl.WC.check(WC.java:466) at org.tigris.subversion.javahl.SVNTests$OneTest.checkStatus(SVNTests.java:851) at org.tigris.subversion.javahl.SVNTests$OneTest.checkStatus(SVNTests.java:833) at org.tigris.subversion.javahl.BasicTests.testBasicRevert(BasicTests.java:1359) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at org.tigris.subversion.javahl.RunTests.main(RunTests.java:116) 2) testBasicDelete(org.tigris.subversion.javahl.BasicTests)junit.framework.AssertionFailedError: removed versioned dir at org.tigris.subversion.javahl.BasicTests.testBasicDelete(BasicTests.java:1640) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at org.tigris.subversion.javahl.RunTests.main(RunTests.java:116) 3) testBasicNodeKindChange(org.tigris.subversion.javahl.BasicTests)junit.framework.AssertionFailedError: can change node kind at org.tigris.subversion.javahl.BasicTests.testBasicNodeKindChange(BasicTests.java:1743) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at org.tigris.subversion.javahl.RunTests.main(RunTests.java:116) FAILURES!!! Tests run: 50, Failures: 3, Errors: 0 The current recursive lock code for multi-db doesn't have stable behavior over added and removed directories, while the single-db code has. I didn't feel like reinventing a complete lock store system like the old access batons just to fix these issues for multi-db. (Single-db has a real recursive lock, so it only has to delete the lock from the root) The recursive lock is a problem for non-recursive revert as it no longer returns SVN_ERR_WC_NOT_LOCKED, see the XFail in revert_tests.py (and our client explicitly checks for that error). We can fix the recursive behaviour of revert, but can we fix the return value? Is it acceptable to return SVN_ERR_WC_NOT_LOCKED when we have a recursive lock? -- Philip
RE: svn commit: r990916 - in /subversion/trunk/subversion/bindings/javahl/native: ClientContext.cpp ClientContext.h
-Original Message- From: Philip Martin [mailto:philip.mar...@wandisco.com] Sent: dinsdag 31 augustus 2010 10:26 To: dev@subversion.apache.org Subject: Re: svn commit: r990916 - in /subversion/trunk/subversion/bindings/javahl/native: ClientContext.cpp ClientContext.h Bert Huijben b...@qqmail.nl writes: (I see a completely different test failure on the Windows bot) On Linux I get the same as Hyrum. Can you run this same test with single-db? In single-db on Linux I get 3 Failures rather than 1 Error: .F..F..F . Time: 139.136 There were 3 failures: 1) testBasicRevert(org.tigris.subversion.javahl.BasicTests)junit.framework .AssertionFailedError: status not found in working copy: A/B/E/alpha at org.tigris.subversion.javahl.WC.check(WC.java:466) at org.tigris.subversion.javahl.SVNTests$OneTest.checkStatus(SVNTests.java :851) at org.tigris.subversion.javahl.SVNTests$OneTest.checkStatus(SVNTests.java :833) at org.tigris.subversion.javahl.BasicTests.testBasicRevert(BasicTests.java :1359) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.ja va:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccesso rImpl.java:43) at org.tigris.subversion.javahl.RunTests.main(RunTests.java:116) This looks like a similar error as the one I see on the Windows buildbot in multi-db. 2) testBasicDelete(org.tigris.subversion.javahl.BasicTests)junit.framework .AssertionFailedError: removed versioned dir at org.tigris.subversion.javahl.BasicTests.testBasicDelete(BasicTests.java :1640) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.ja va:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccesso rImpl.java:43) at org.tigris.subversion.javahl.RunTests.main(RunTests.java:116) This might be checking multi-db behavior. It looks like it just deletes a directory and expects missing data. (By just looking at the description) 3) testBasicNodeKindChange(org.tigris.subversion.javahl.BasicTests)junit.f ramework.AssertionFailedError: can change node kind at org.tigris.subversion.javahl.BasicTests.testBasicNodeKindChange(BasicTe sts.java:1743) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.ja va:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccesso rImpl.java:43) at org.tigris.subversion.javahl.RunTests.main(RunTests.java:116) And changing node kind is no problem with single-db. FAILURES!!! Tests run: 50, Failures: 3, Errors: 0 snip Rest of the response in a different thread. Bert
RE: Recursive revert behavior (was: svn commit: r990916 )
-Original Message- From: Philip Martin [mailto:philip.mar...@wandisco.com] Sent: dinsdag 31 augustus 2010 10:26 To: dev@subversion.apache.org Subject: Re: svn commit: r990916 - in /subversion/trunk/subversion/bindings/javahl/native: ClientContext.cpp ClientContext.h Bert Huijben b...@qqmail.nl writes: snip See original thread The current recursive lock code for multi-db doesn't have stable behavior over added and removed directories, while the single-db code has. I didn't feel like reinventing a complete lock store system like the old access batons just to fix these issues for multi-db. (Single-db has a real recursive lock, so it only has to delete the lock from the root) The recursive lock is a problem for non-recursive revert as it no longer returns SVN_ERR_WC_NOT_LOCKED, see the XFail in revert_tests.py (and our client explicitly checks for that error). We can fix the recursive behaviour of revert, but can we fix the return value? Is it acceptable to return SVN_ERR_WC_NOT_LOCKED when we have a recursive lock? This is one of the revert tests if I remember correctly? I think we have to move these checks in the revert code instead of forcing old lock behavior. But I really don't know what we should do here, except for maybe revving svn_client_revertX(), to move the old behavior in the deprecated wrapper. For who doesn't know the full story: This is about * svn revert root-of-copy-operation vs * svn revert -R root-of-copy-operation With the new fully recursive lock behavior in all libsvn_client functions this 'svn revert WC-TREE' just works, but it is a big behavior change. And in this case the old behavior warned you before performing a lossy operation, which was kind of useful. For new code I think this kind of checks belong in the client (svn, TortoiseSVN, AnkhSVN, etc.) instead of in libsvn_wc or libsvn_client. But in this case it changes a lot of old code. Bert
Re: Recursive revert behavior
Bert Huijben b...@qqmail.nl writes: This is one of the revert tests if I remember correctly? Yes. I think we have to move these checks in the revert code instead of forcing old lock behavior. But I really don't know what we should do here, except for maybe revving svn_client_revertX(), to move the old behavior in the deprecated wrapper. For who doesn't know the full story: This is about * svn revert root-of-copy-operation vs * svn revert -R root-of-copy-operation With the new fully recursive lock behavior in all libsvn_client functions this 'svn revert WC-TREE' just works, but it is a big behavior change. And in this case the old behavior warned you before performing a lossy operation, which was kind of useful. For new code I think this kind of checks belong in the client (svn, TortoiseSVN, AnkhSVN, etc.) instead of in libsvn_wc or libsvn_client. But in this case it changes a lot of old code. At present svn_wc_revert4(root_of_copy, depth=empty) succeeds and recursively reverts the copied tree. In 1.6 svn_wc_revert3(root_of_copy, depth=empty) would fail with SVN_ERR_WC_NOT_LOCKED if the copied directory had versioned subdirectories. (I've just realised that 1.6 is inconsistent, it only errors on children that are directories not children that are files.) We could make revert with depth=empty fail on a copied tree unless the tree is empty. If we do that it would probably fail with some sort of depth error. I suppose svn_wc_revert3 could convert that to SVN_ERR_WC_NOT_LOCKED for 1.6 compatibility? -- Philip
RE: svn commit: r990916 - in /subversion/trunk/subversion/bindings/javahl/native: ClientContext.cpp ClientContext.h
-Original Message- From: hy...@hyrumwright.org [mailto:hy...@hyrumwright.org] On Behalf Of Hyrum Wright Sent: maandag 30 augustus 2010 21:43 To: Subversion Development Cc: commits Subject: Re: svn commit: r990916 - in /subversion/trunk/subversion/bindings/javahl/native: ClientContext.cpp ClientContext.h On Mon, Aug 30, 2010 at 2:37 PM, hwri...@apache.org wrote: Author: hwright Date: Mon Aug 30 19:37:05 2010 New Revision: 990916 URL: http://svn.apache.org/viewvc?rev=990916view=rev Log: JavaHL: Create a persistent C client context, in order to reuse the working copy context between client API invocations. Note: This introduces a test failure which I believe to be a manifestation of a bug in wc-ng. For the interested, this is the test failure: [[[ hwri...@orac:~/dev/svn-trunk2$ make check-javahl /usr/lib/jvm/java-6-openjdk//bin/java -Dtest.rootdir=/home/hwright/dev/svn- trunk2/subversion/bindings/javahl/test-work -Dtest.srcdir=/home/hwright/dev/svn-trunk2/subversion/bindings/javahl -Dtest.rooturl= -Dtest.fstype= - Djava.library.path=subversion/bindings/javahl/native/.libs:/usr/local/l ib -classpath subversion/bindings/javahl/classes:/home/hwright/dev/svn- trunk2/subversion/bindings/javahl/src:/usr/share/java/junit.jar -Dtest.tests= org.apache.subversion.javahl.RunTests ..E... ... Time: 26.083 There was 1 error: 1) testBasicRevert(org.apache.subversion.javahl.BasicTests)org.apache.subv ersion.javahl.ClientException: Attempted to lock an already-locked dir svn: Working copy '/home/hwright/dev/svn-trunk2/subversion/bindings/javahl/test- work/working_copies/basic_test19/X' locked SQLite error svn: disk I/O error svn: disk I/O error svn: cannot rollback - no transaction is active Working copy not locked; this is probably a bug, please report svn: Working copy not locked at '/home/hwright/dev/svn-trunk2/subversion/bindings/javahl/test- work/working_copies/basic_test19/X'. at native.subversion.libsvn_wc(wc_db.c:8481) at native.subversion.libsvn_subr(sqlite.c:118) at native.subversion.libsvn_subr(sqlite.c:513) at native.subversion.libsvn_subr(sqlite.c:211) at native.subversion.libsvn_wc(wc_db.c:8276) at native.subversion.libsvn_subr(sqlite.c:118) at native.subversion.libsvn_wc(lock.c:1655) at native.subversion.libsvn_wc(lock.c:1639) at native.subversion.libsvn_wc(lock.c:1744) at native.subversion.libsvn_wc(lock.c:1844) at native.subversion.libsvn_client(revert.c:176) at org.apache.subversion.javahl.SVNClient.revert(Native Method) at org.apache.subversion.javahl.BasicTests.testBasicRevert(BasicTests.java :1332) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.ja va:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccesso rImpl.java:43) at org.apache.subversion.javahl.RunTests.main(RunTests.java:116) FAILURES!!! Tests run: 52, Failures: 0, Errors: 1 make: *** [check-javahl] Error 1 hwri...@orac:~/dev/svn-trunk2$ ]]] I believe it to be a legitimate bug, not only because the client APIs are being used in a completely reasonable way, but also because the output told me so. :) The problem with this test is that we still have the X/.svn/wc.db handle open while we delete X (referenced from the pdh's wcroot). (This test does this to try some missing directory behavior). We then try to store a lock in wc.db (to perform the revert), which makes SQLite create a transaction log file next to wc.db. But then it finds that this directory no longer exists, so it can't create the file. On Windows the directory is not deleted because we still have that file open. (File.delete() returns false in SVNTest.removeDirOrFile(), but we don't check the result there). But as this doesn't give the expected result (X is unmodified there), the test fails a few lines later. This specific issue is fixed once we move to SingleDB, but the problem that we can delete databases that we have open will stay (on non Windows systems). Bert
Re: FSFS error in DAV MERGE - Can't open file 'db/transactions/props'
I have opened issue #3696, http://subversion.tigris.org/issues/show_bug.cgi?id=3696, to track this. - Julian On Fri, 2010-07-30 at 16:17 +0100, Julian Foad wrote: On Tue, 2010-06-22 at 13:57 +0100, Julian Foad wrote: I am investigating the following DAV + FSFS server error message which we at WANdisco have seen rarely, but in at least two real installations, and am sharing it here in case anyone can shed light on it. It happens during a DAV 'MERGE': Can't open file '[...]/db/transactions/props': No such file or directory FSFS is trying to open an invalid path; it should be '[...]/db/transactions/txn-id/props'. From the source code, I can see how this must happen at a low level: the txn-id path component will be omitted if and only if something passes a NULL transaction-id string to fs_fs.c:get_txn_proplist(). But why and from where? A status update: I've traced the calls to get_txn_proplist(), a few call levels up, and I believe one of these must have supplied txn_id = NULL. Callees of get_txn_proplist(), with txn-id parameter identified by name and by param-list position: get_txn_proplist(,,txn_id) svn_fs_fs__change_txn_props(txn-id) # DOESN'T CAUSE THE ERROR txn_vtable.change_props(txn-id) svn_fs_fs__change_txn_prop(txn-id) commit_body() svn_fs_fs__begin_txn(using svn_fs_fs__create_txn()) svn_fs_fs__begin_obliteration_txn(using ...) svn_fs_fs__get_txn(txn_id)# CAN CAUSE THE ERROR svn_fs_fs__open_txn(,,name) svn_fs_t.vtable.open_txn(,,name) svn_fs_open_txn(,,name) dav_svn__abort_txn(,txn_name) prep_working() # NO, txn_name != NULL prep_private(comb-priv.root.txn_name) DAV_RESOURCE_TYPE_PRIVATE is_our_resource() # NO, txn name != NULL open_txn(,,txn_name) dav_svn__checkout() merge() dav_svn__hooks_vsn.merge fs_merge(,,,target_root-txn) svn_fs_root_t.vtable.merge(,,,target_root-txn) svn_fs_merge(,,target_root-txn) # no callers? svn_fs_fs__get_txn_ids(,,,txn_name) svn_fs_fs__dag_txn_root(,,txn_id) root_node-open_path-get_dag(,root-txn) svn_fs_fs__dag_txn_base_root(,,txn_id) merge_changes(,,txn-id) svn_fs_fs__commit_txn(,,txn-id) svn_fs_txn_t.vtable.commit(,,txn-id) svn_fs_commit_txn(,,txn-id) fs_merge() svn_fs_fs__dag_clone_root(,,txn_id) mutable_root_node(,root-txn) make_path_mutable(root-txn) commit_body(baton-txn-id) # DOESN'T CAUSE THE ERROR svn_fs_fs__commit(,,txn-id) svn_fs_fs__commit_txn(,,txn-id) svn_fs_fs__txn_proplist(,txn-id) # CAN CAUSE THE ERROR svn_fs_txn_t.vtable.get_proplist(,txn-id) svn_fs_txn_proplist(,,txn-id) commit_body() svn_fs_fs__txn_prop(,txn-id) svn_fs_txn_t.vtable.get_prop(,txn-id) svn_fs_txn_prop(,txn-id) svn_fs_fs__txn_root(,txn-id) svn_fs_txn_t.vtable.root(,txn-id) svn_fs_txn_root(,txn-id) The ones annotated CAN CAUSE THE ERROR did result in the Can't open 'db/transactions/props' error when I forced txn_id=NULL at these places in a trivial test scenario, and the ones marked DOESN'T, didn't. From the fact that a DAV MERGE was the operation under way, I would hazard a guess that dav_svn__hooks_vsn.merge() might be the call that's going wrong. I haven't tried to trace that back to its caller (in mod_dav perhaps?). I don't know if this line of attack has any mileage left in it. I'm thinking of leaving it at that for now, unless anyone comes up with suggestions for how to proceed. I don't feel capable of setting up a test system that would replicate the sequence of DAV messages that resulted in the error condition. (The issue of the current assertion leaving a DOS attack possible is still in my sights - email thread Re: svn commit: r980046.) - Julian The error might be triggered by the WANdisco software sending commands in the wrong sequence, or something like that, and so to that extent it may not be interesting to the general community. In one case, detailed below, it happened after a DELETE was sent, more than an hour into a very long commit. In another case, it happened after a start-commit hook failed. The server might be receiving a DAV MERGE request when it isn't expecting one. Can anyone comment on that, given this limited information? Here are some Apache access log entries from one occurrence, with paths anonymized and the client Id string (always SVN/1.6.6 (r40053) neon/0.28.3) omitted for brevity: [[[ 127.0.0.1 - UserK [16/Dec/2009:20:25:17 -0800] MKACTIVITY /svn/RepoJ/!svn/act/ActivityA HTTP/1.1 201 312 - 127.0.0.1 - UserK [16/Dec/2009:23:20:45 -0800] PROPPATCH /svn/RepoJ/!svn/wrk/ActivityA/branches/BranchD/PathG HTTP/1.1 207 582 -
Re: Recursive revert behavior
On 08/31/2010 05:10 AM, Bert Huijben wrote: -Original Message- From: Philip Martin [mailto:philip.mar...@wandisco.com] Sent: dinsdag 31 augustus 2010 10:26 To: dev@subversion.apache.org Subject: Re: svn commit: r990916 - in /subversion/trunk/subversion/bindings/javahl/native: ClientContext.cpp ClientContext.h Bert Huijben b...@qqmail.nl writes: snip See original thread The current recursive lock code for multi-db doesn't have stable behavior over added and removed directories, while the single-db code has. I didn't feel like reinventing a complete lock store system like the old access batons just to fix these issues for multi-db. (Single-db has a real recursive lock, so it only has to delete the lock from the root) The recursive lock is a problem for non-recursive revert as it no longer returns SVN_ERR_WC_NOT_LOCKED, see the XFail in revert_tests.py (and our client explicitly checks for that error). We can fix the recursive behaviour of revert, but can we fix the return value? Is it acceptable to return SVN_ERR_WC_NOT_LOCKED when we have a recursive lock? This is one of the revert tests if I remember correctly? I think we have to move these checks in the revert code instead of forcing old lock behavior. But I really don't know what we should do here, except for maybe revving svn_client_revertX(), to move the old behavior in the deprecated wrapper. For who doesn't know the full story: This is about * svn revert root-of-copy-operation vs * svn revert -R root-of-copy-operation With the new fully recursive lock behavior in all libsvn_client functions this 'svn revert WC-TREE' just works, but it is a big behavior change. And in this case the old behavior warned you before performing a lossy operation, which was kind of useful. I think requiring -R for any deep revert makes sense and is the behavior that users have grown to expect. But why not at least make 'svn revert COPY-TARGET-DIR' do something useful, such as revert any propchanges made to the copy target? It can even still notify with a warning that the reversion wasn't deep, suggesting the --recursive flag as we do today. In other words, where today we see: $ svn copy A A-copy $ svn pset foo bar A-copy $ svn revert A-copy svn: Try 'svn revert --depth infinity' instead? subversion/libsvn_wc/lock.c:952: (apr_err=155005) svn: Unable to lock 'A-copy/B' $ We might see: $ svn copy A A-copy $ svn pset foo bar A-copy $ svn revert A-copy Reverted property changes for 'Z' svn: warning: Directory 'Z' contains additional changes; use 'svn \ revert --depth infinity' to revert the whole tree. $ Just a thought. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: svn commit: r980046 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
On Wed, 2010-07-28, Julian Foad wrote: On Wed, 2010-07-28 at 10:13 -0700, Blair Zajac wrote: On 07/28/2010 06:18 AM, julianf...@apache.org wrote: Author: julianfoad Date: Wed Jul 28 13:18:28 2010 New Revision: 980046 URL: http://svn.apache.org/viewvc?rev=980046view=rev Log: Add assertions in FSFS to trap an internal error that is believed to have occurred in real life. Propose this one for a 1.6.x backport? I don't think so. It doesn't provide an enhanced experience for the user. Quite the opposite, in fact - as Bert pointed out, if the server terminates because of this error, that can be worse than it failing a commit and returning the error message to the client. So these assertions are more to help us spot the location of the bug more quickly if we ever manage to reproduce the problem in a test environment, and to help us avoid writing any more silly bugs of this kind. Regarding Bert's concern about DOS attacks: if we can't find and fix the bug that leads to this condition then I suppose we should add a friendly error message instead of the assertion, and a comment explaining why it's not just a simple assertion. Done in r991182. Now *this* error handling should be back-ported to 1.6.x. - Julian
Enabling Single-DB mode today
We want to get to single-DB mode ASAP. Is anything still blocking us from enabling it today? * One or two test failures? - I see stat_tests 5 and 6 failing at r991181. * Upgrade from 1.6? - I don't think this should block it. * Upgrade from 1.7-dev? - The Py script seems sufficient. I see no blockers after the stat tests are fixed, so I intend to enable it then. Any objections? - Julian
RE: Enabling Single-DB mode today
-Original Message- From: Julian Foad [mailto:julian.f...@wandisco.com] Sent: dinsdag 31 augustus 2010 15:41 To: dev@subversion.apache.org Subject: Enabling Single-DB mode today We want to get to single-DB mode ASAP. Is anything still blocking us from enabling it today? * One or two test failures? - I see stat_tests 5 and 6 failing at r991181. Stat 5 is fixed in r991188, stat 6 needs some expected result updates (See irc log) * Upgrade from 1.6? - I don't think this should block it. Last I heard was that at least our regression tests pass. But I think this code needs a lot more cleanup before 1.7. (But it shouldn't hold this step) * Upgrade from 1.7-dev? - The Py script seems sufficient. I think we already discussed this on list. I see no blockers after the stat tests are fixed, so I intend to enable it then. Any objections? +1 All tests Pass or XPass on Windows with single-db, except for the upgrade tests. (Philip is working on this one) Bert
Re: Enabling Single-DB mode today
On Tue, Aug 31, 2010 at 9:40 AM, Julian Foad julian.f...@wandisco.com wrote: We want to get to single-DB mode ASAP. Is anything still blocking us from enabling it today? * One or two test failures? - I see stat_tests 5 and 6 failing at r991181. * Upgrade from 1.6? - I don't think this should block it. * Upgrade from 1.7-dev? - The Py script seems sufficient. I see no blockers after the stat tests are fixed, so I intend to enable it then. Any objections? Do it! It's felt like we have two versions of trunk lately :-) - Julian
Re: Enabling Single-DB mode today
On 08/31/2010 09:51 AM, Paul Burba wrote: On Tue, Aug 31, 2010 at 9:40 AM, Julian Foad julian.f...@wandisco.com wrote: We want to get to single-DB mode ASAP. Is anything still blocking us from enabling it today? * One or two test failures? - I see stat_tests 5 and 6 failing at r991181. * Upgrade from 1.6? - I don't think this should block it. * Upgrade from 1.7-dev? - The Py script seems sufficient. I see no blockers after the stat tests are fixed, so I intend to enable it then. Any objections? Do it! It's felt like we have two versions of trunk lately :-) +1! -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: Enabling Single-DB mode today
On 31.08.2010 15:51, Paul Burba wrote: On Tue, Aug 31, 2010 at 9:40 AM, Julian Foad julian.f...@wandisco.com wrote: We want to get to single-DB mode ASAP. Is anything still blocking us from enabling it today? * One or two test failures? - I see stat_tests 5 and 6 failing at r991181. * Upgrade from 1.6? - I don't think this should block it. * Upgrade from 1.7-dev? - The Py script seems sufficient. I see no blockers after the stat tests are fixed, so I intend to enable it then. Any objections? Do it! It's felt like we have two versions of trunk lately :-) I might even start building trunk again if you do. :) -- Brane
Re: Enabling Single-DB mode today
Bindings: swig-py, swig-pl, swig-rb all pass their tests. ctypes-python fails to build for me. Java: Philip said I've run javahl tests in single-db, 3 FAILs compared to 1 in multi-db. Upgrading from a dev version: When the switch is made, after updating and re-compiling, each dev using a trunk WC (format 18) will need to run tools/dev/wc-ng/bump-to-19.py on the WC. Running the new svn on an format-18 WC will report a friendly message advising you to use bump-to-19.py. (Running it on an older 1.7-dev (format = 17) WC will probably upgrade one directory to format 18 and then exit with that message, which doesn't sound quite right.) Running an old svn on a format-19 (single-db) WC results in a friendly error. I think we're all set to switch it on. - Julian
Re: Enabling Single-DB mode today
On 08/31/2010 10:36 AM, Julian Foad wrote: Bindings: swig-py, swig-pl, swig-rb all pass their tests. ctypes-python fails to build for me. Java: Philip said I've run javahl tests in single-db, 3 FAILs compared to 1 in multi-db. Upgrading from a dev version: When the switch is made, after updating and re-compiling, each dev using a trunk WC (format 18) will need to run tools/dev/wc-ng/bump-to-19.py on the WC. Running the new svn on an format-18 WC will report a friendly message advising you to use bump-to-19.py. (Running it on an older 1.7-dev (format = 17) WC will probably upgrade one directory to format 18 and then exit with that message, which doesn't sound quite right.) Running an old svn on a format-19 (single-db) WC results in a friendly error. I think we're all set to switch it on. Ooh. Wait a second. I seem to recall that your Python script had some limitations -- couldn't handle externals, or something. Is that still the case? Can it be worked around by something like: 1. rm -rf /path/to/external/directory 2. bump-to-19.py 3. svn up ? -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: Enabling Single-DB mode today
On Tue, 2010-08-31 at 10:41 -0400, C. Michael Pilato wrote: On 08/31/2010 10:36 AM, Julian Foad wrote: Bindings: swig-py, swig-pl, swig-rb all pass their tests. ctypes-python fails to build for me. Java: Philip said I've run javahl tests in single-db, 3 FAILs compared to 1 in multi-db. Upgrading from a dev version: When the switch is made, after updating and re-compiling, each dev using a trunk WC (format 18) will need to run tools/dev/wc-ng/bump-to-19.py on the WC. Running the new svn on an format-18 WC will report a friendly message advising you to use bump-to-19.py. (Running it on an older 1.7-dev (format = 17) WC will probably upgrade one directory to format 18 and then exit with that message, which doesn't sound quite right.) Running an old svn on a format-19 (single-db) WC results in a friendly error. I think we're all set to switch it on. Ooh. Wait a second. I seem to recall that your Python script had some limitations -- couldn't handle externals, or something. Is that still the case? Can it be worked around by something like: 1. rm -rf /path/to/external/directory 2. bump-to-19.py 3. svn up ? Yes, in fact it's essential to do that because otherwise the script would make the contents of each external WC become an integral part of the main WC, because it will just recurse into them and move their DB rows into the main WC's root DB. - Julian
Re: Enabling Single-DB mode today
On Tue, Aug 31, 2010 at 10:36 AM, Julian Foad julian.f...@wandisco.com wrote: Bindings: swig-py, swig-pl, swig-rb all pass their tests. ctypes-python fails to build for me. Java: Philip said I've run javahl tests in single-db, 3 FAILs compared to 1 in multi-db. When I ran JavaHL tests a week or so ago in single-db mode the only failing tests were ones where the test expected a local directory delete to leave the directory on disk. So we should just have to adjust the test expectations after conversion. +1 from me on the upgrade. -- Thanks Mark Phippard http://markphip.blogspot.com/
Re: Enabling Single-DB mode today
On Tue, Aug 31, 2010 at 9:38 AM, Mark Phippard markp...@gmail.com wrote: On Tue, Aug 31, 2010 at 10:36 AM, Julian Foad julian.f...@wandisco.com wrote: Bindings: swig-py, swig-pl, swig-rb all pass their tests. ctypes-python fails to build for me. Java: Philip said I've run javahl tests in single-db, 3 FAILs compared to 1 in multi-db. When I ran JavaHL tests a week or so ago in single-db mode the only failing tests were ones where the test expected a local directory delete to leave the directory on disk. So we should just have to adjust the test expectations after conversion. Mark, will you update the JavaHL test expectations when the switch-over occurs? (Note that there is still one failure due to r990916.) +1 from me on the upgrade. +1 from me as well. -Hyrum
Re: Enabling Single-DB mode today
Enabled in r991236. We can leave the #defines in for a little while (like a few days, stsp and I suggest) to facilitate review and comparative testing. - Julian
WARNING: WC format bumped to 19: single-DB enabled: TAKE CARE
BACK UP YOUR WC before proceeding. When you upgrade to r991236 or later, single-DB (format 19) is enabled. If you're running trunk and using it for your WCs, you'll need to upgrade those WCs before svn will let you continue to use them. Currently the upgrade from format 18 is performed by tools/dev/wc-ng/bump-to-19.py. CAUTION: It's just a hacky script and it has some very rough edges. In particular, it will assimilate any versioned directory, *including things like externals that it shouldn't touch*. If you have any externals, you must delete them (the populated directories, not the properties) before running the script, and then run svn update to get them back again afterwards. - Julian
Re: WARNING: WC format bumped to 19: single-DB enabled: TAKE CARE
Improvements to bump-to-19.py are flying in... get the latest version before you use it. If it fails in the deleting ... phase, it's done the migration work so you should be able to recover simply by deleting all remaining .svn dirs manually (carefully avoiding the root one of course.) - Julian On Tue, 2010-08-31 at 18:02 +0100, Julian Foad wrote: BACK UP YOUR WC before proceeding. When you upgrade to r991236 or later, single-DB (format 19) is enabled. If you're running trunk and using it for your WCs, you'll need to upgrade those WCs before svn will let you continue to use them. Currently the upgrade from format 18 is performed by tools/dev/wc-ng/bump-to-19.py. CAUTION: It's just a hacky script and it has some very rough edges. In particular, it will assimilate any versioned directory, *including things like externals that it shouldn't touch*. If you have any externals, you must delete them (the populated directories, not the properties) before running the script, and then run svn update to get them back again afterwards. - Julian
Re: WARNING: WC format bumped to 19: single-DB enabled: TAKE CARE
Julian Foad julian.f...@wandisco.com writes: CAUTION: It's just a hacky script and it has some very rough edges. In particular, it will assimilate any versioned directory, *including things like externals that it shouldn't touch*. If you have any externals, you must delete them (the populated directories, not the properties) before running the script, and then run svn update to get them back again afterwards. I've tweaked the script so that it should remain in a single wc and skip any externals or nested wcs. Each external should be upgrade separately. -- Philip
Re: WARNING: WC format bumped to 19: single-DB enabled: TAKE CARE
Philip Martin wrote: Julian Foad julian.f...@wandisco.com writes: CAUTION: It's just a hacky script and it has some very rough edges. In particular, it will assimilate any versioned directory, *including things like externals that it shouldn't touch*. If you have any externals, you must delete them (the populated directories, not the properties) before running the script, and then run svn update to get them back again afterwards. I've tweaked the script so that it should remain in a single wc and skip any externals or nested wcs. Each external should be upgrade separately. Thank you immensely, Philip. - Julian
Re: svn commit: r990537 - /subversion/branches/performance/subversion/libsvn_subr/svn_temp_serializer.c
On Tue, Aug 31, 2010 at 12:37 AM, Stefan Fuhrmann stefanfuhrm...@alice-dsl.de wrote: Johan Corveleyn wrote: On Mon, Aug 30, 2010 at 9:32 PM, Johan Corveleyn jcor...@gmail.commailto: jcor...@gmail.com wrote: On Mon, Aug 30, 2010 at 12:05 PM, Stefan Fuhrmann stefanfuhrm...@alice-dsl.de mailto:stefanfuhrm...@alice-dsl.de wrote: I thought so ;) To narrow down the nature of the problem, I added some checks that should be able to discern plain data corruption from (de-)serialization issues. Please apply either the patch or replace the original files with the versions in the .zip file. A debug build should then, hopefully, trigger a different and more specific assertion. Ok, will try that now. :-(, I updated to r990759 and applied your attached debug.patch, but I just get the same assertion (offset by 4 lines), with no additional information: [[[ $ svnserve -d -r c:/research/svn/experiment/repos Assertion failed: *ptr buffer, file ..\..\..\subversion\libsvn_subr\svn_temp_serializer.c, line 286 ]]] Anything else I can try? The failure was expected. With the patched code, I just hope to get more context information. Could you send me the call stack dump? My suspicion is that the DAG node is already (partially) garbage before it actually gets added to the cache. Ok, here it is in attachment (I removed the assert that you added in this commit, so that the app would really crash; otherwise I couldn't get a crash dump (or can I?)). (I hope the attachments (file svn-crash...log.txt and svn-crash...dmp) get through; if not, let me know and I'll zip them or something) Some additional info: - I couldn't reproduce the crash with a narrow range. Not even 9:0 would crash it (right after startup). - BUT: if after 9:0 I run log without -r arguments, I get an error on the client side: [[[ ..\..\..\subversion\svn\log-cmd.c:746: (apr_err=160013) ..\..\..\subversion\libsvn_client\log.c:606: (apr_err=160013) ..\..\..\subversion\libsvn_repos\log.c:1474: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_repos\log.c:372: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_fs_fs\tree.c:3313: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_fs_fs\tree.c:3313: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_fs_fs\tree.c:3159: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_fs_fs\tree.c:668: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ]]] - This also happens when the first run is 6:0 or even 42000:0. If the first run is 41000:0, then the second run doesn't get the client-side error, but the server crashes on the expected spot (after rev 42100). - The above client-side error also happens if the second run is 96000:9 instead of a log without -r argument. - However, if I run log -r96000:9 right after startup, no problem. - Other than that, it crashes reproducibly after 42100 if I run log with no -r arguments right after startup. Hmm, I hope you can figure this out ... -- Johan Process info: Cmd line: C:\research\svn\client_build\svn_branch_performance\dist\bin\svnserve.exe -d -r c:/research/svn/experiment/repos Working Dir: C:\research\svn\experiment\repos Version: 1.7.0 (dev build), compiled Aug 19 2010, 00:50:40 Platform: Windows OS version 5.1 build 2600 Service Pack 3 Exception: ACCESS_VIOLATION Registers: eax=de12e56f ebx=00d31370 ecx=00dd3573 edx=00dd3573 esi=00f0f7c4 edi=00f0f810 eip=003baffc esp=00f0f784 ebp=00f0f784 efl=00010286 cs=001b ss=0023 ds=0023 es=0023 fs=003b gs= Stacktrace: #1 0x003baffc in svn_fs_fs__id_deserialize() buffer = 0x00dd356f id = #2 0x003bb598 in svn_fs_fs__noderev_deserialize() buffer = 0x00dd3570 noderev_p = noderev = #3 0x003bfa4e in svn_fs_fs__dag_deserialize() out = data = data_len = 1e0 pool = node = #4 0x004bc6fa in membuffer_cache_get() cache = key = key_len = 31 item = deserializer = pool = buffer = 0x00dd3570 group_index = 6587 to_find = entry = size = 1e0 #5 0x004bc3b4 in svn_membuffer_cache_get() svn_err__temp = value_p = found = 0x00f0f8c8 cache_void = 0x00d6bc40 key = pool = full_key = 0x00dd3520 cache = full_key_len = 31 #6 0x004bdf93 in svn_cache__get() value_p = found = 0x00f0f8c8 cache = key = pool = #7 0x003b1158 in dag_node_cache_get() svn_err__temp = node_p = root = path = pool = node = cache = key = found = 0 #8
Re: svn commit: r990537 - /subversion/branches/performance/subversion/libsvn_subr/svn_temp_ser ializer.c
Johan Corveleyn wrote on Tue, Aug 31, 2010 at 22:07:59 +0200: (I hope the attachments (file svn-crash...log.txt and svn-crash...dmp) get through; if not, let me know and I'll zip them or something) I see svn-crash-log20100831213059.log.txt only. By the way, please send as plain text, if possible. (you sent multipart/alternative with an html part)
Re: WARNING: WC format bumped to 19: single-DB enabled: TAKE CARE
Where was the heads up or the discussion to do this now? On Tue, Aug 31, 2010 at 13:02, Julian Foad julian.f...@wandisco.com wrote: BACK UP YOUR WC before proceeding. When you upgrade to r991236 or later, single-DB (format 19) is enabled. If you're running trunk and using it for your WCs, you'll need to upgrade those WCs before svn will let you continue to use them. Currently the upgrade from format 18 is performed by tools/dev/wc-ng/bump-to-19.py. CAUTION: It's just a hacky script and it has some very rough edges. In particular, it will assimilate any versioned directory, *including things like externals that it shouldn't touch*. If you have any externals, you must delete them (the populated directories, not the properties) before running the script, and then run svn update to get them back again afterwards. - Julian
Re: svn commit: r990537 - /subversion/branches/performance/subversion/libsvn_subr/svn_temp_serializer.c
Stefan, On Tue, Aug 31, 2010 at 10:07 PM, Johan Corveleyn jcor...@gmail.com wrote: Some additional info: - I couldn't reproduce the crash with a narrow range. Not even 9:0 would crash it (right after startup). - BUT: if after 9:0 I run log without -r arguments, I get an error on the client side: [[[ ..\..\..\subversion\svn\log-cmd.c:746: (apr_err=160013) ..\..\..\subversion\libsvn_client\log.c:606: (apr_err=160013) ..\..\..\subversion\libsvn_repos\log.c:1474: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_repos\log.c:372: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_fs_fs\tree.c:3313: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_fs_fs\tree.c:3313: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_fs_fs\tree.c:3159: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ..\..\..\subversion\libsvn_fs_fs\tree.c:668: (apr_err=160013) svn: File not found: revision 90799, path '?\239' ]]] - This also happens when the first run is 6:0 or even 42000:0. If the first run is 41000:0, then the second run doesn't get the client-side error, but the server crashes on the expected spot (after rev 42100). - The above client-side error also happens if the second run is 96000:9 instead of a log without -r argument. - However, if I run log -r96000:9 right after startup, no problem. - Other than that, it crashes reproducibly after 42100 if I run log with no -r arguments right after startup. I experimented some more. There must be something strange with that revision 90799 that also causes the client-side error. Some log runs immediately after startup: - svn log -r90798:0 svn://localhost/path/bigfile.xml: no crash - svn log -r90799:0 svn://localhost/path/bigfile.xml: crash (last log entry: 42104 (one before 42100 of the regular crash)) - svn log -r90921:0 svn://localhost/path/bigfile.xml: crash (last log entry: 42130 (two before 42100 of the regular crash)). r90921 is one before 90799. - svn log -r90998:0 svn://localhost/path/bigfile.xml: crash (last log entry: 42149 (three before 42100 of the regular crash)). r90998 is two before 90799. - svn log svn://localhost/path/bigfile.xml: still crashes consistently with last log entry 42100. Still r90799 itself seems a very normal commit, with only text modifications to bigfile.xml. One more note: the repository was once created by converting our CVS repository with cvs2svn (it's an old conversion that we did as an experiment, after which we did the real conversion; but I still use the old converted repo to test things). I just now notice that we did that old conversion with the cvs-revnum option, i.e. updating the cvs2svn:cvs-rev property on every commit, to make it contain the cvs revision number of the file. So every commit also contains prop changes. Probably not relevant, but you never know :-). Cheers, -- Johan
Another FSFS bug somewhere?
I can't be sure which version of SVN this occurred with (I believe it was a very recent version), but I had a user email me the other day about a broken revision. After taking a look, it appears that SVN picked the right offset, the right length, and the right checksums, but the wrong revision number. It looks like this was during a tag operation (or a copy from a previous revision). The revision the backend chose didn't even have a related file, and the contents certainly were not the same. Any ideas where I should look to find the cause of this problem? Also, I noticed that files seem to have both a SHA-1 and a uniquifier, but directories don't. The structure document in libsvn_fs_fs doesn't speak of this difference between files and directories. It just says that newer formats have it while older ones don't. Should directories have both the SHA-1 and unquifier? Thanks! -John
Re: WARNING: WC format bumped to 19: single-DB enabled: TAKE CARE
sarcasm-mode You mean the note sent three whole hours before this thread's original post? / On Tue, Aug 31, 2010 at 17:48, Mark Phippard markp...@gmail.com wrote: It was in the Enabling SINGLE_DB thread on this list. On Tue, Aug 31, 2010 at 5:30 PM, Greg Stein gst...@gmail.com wrote: Where was the heads up or the discussion to do this now? On Tue, Aug 31, 2010 at 13:02, Julian Foad julian.f...@wandisco.com wrote: BACK UP YOUR WC before proceeding. When you upgrade to r991236 or later, single-DB (format 19) is enabled. If you're running trunk and using it for your WCs, you'll need to upgrade those WCs before svn will let you continue to use them. Currently the upgrade from format 18 is performed by tools/dev/wc-ng/bump-to-19.py. CAUTION: It's just a hacky script and it has some very rough edges. In particular, it will assimilate any versioned directory, *including things like externals that it shouldn't touch*. If you have any externals, you must delete them (the populated directories, not the properties) before running the script, and then run svn update to get them back again afterwards. - Julian -- Thanks Mark Phippard http://markphip.blogspot.com/
Re: RFC: How should Subversion handle OS-deleted paths?
On Thu, Aug 26, 2010 at 8:07 AM, Julian Foad julian.f...@wandisco.com wrote: On Wed, 2010-08-25, Paul Burba wrote: On Tue, Aug 24, 2010 at 7:25 AM, Julian Foad julian.f...@wandisco.com wrote: [...] Agreed, we simply can't be sure what the user's intention was...I'm beginning to be swayed to the idea that restoring the missing subtrees may not be the ideal approach... [...] The pattern that's emerging from my thoughts is: throw an error if logically we need to access the missing working version of the node. If we don't need to access it, just let it be. Never restore it unless the user specifically requests so and says what kind of restoration is required. For these reasons I think merge should also throw an error if it needs to merge changes into a missing node. (If instead it needs to delete it, then it has the options I mentioned for svn delete.) But I suspect part of the reason why restore seems an attractive option is because Subversion isn't very friendly when merge stops with an error. We don't provide any way to resume the same merge, Quite right about the unfriendliness. Resuming the merge is really a hit-or-miss proposition, depending on how much of the merge was done successfully prior to encountering the missing subtree. If it encountered early, before the application of any diffs, then things are ok, but after that things get ugly fast, particularly if the merge target originally had any local mods. It's worth noting again there there are *two* error-out approaches: i) Throw an error when a subtree the editor needs is found to be missing. This is what you are talking about here. ii) Identify any missing subtrees at the *start* of the merge and throw an error before any editor drives are done. Basically we disallow merges with missing subtrees due to OS-level deletes. You're right, I hadn't thought of erroring out before starting the merge, and that is a much, much better option than erroring out in the middle. and we don't make it particularly easy to roll back the merge (although that's getting better now that revert is, I hope, going to remove nodes that it created by copying), and we don't have any way at all to roll back a merge performed in a non-clean WC. So we're trying to avoid erroring out. Long term, those difficult problems are are the problems we should be looking to solve. Short term, I suppose it's useful to avoid erroring out as much as we can. Equally bad is the case with trunk right now, where we simply ignore missing directories, even if the merge would affect them - see http://subversion.tigris.org/issues/show_bug.cgi?id=2915#desc4 for an example. If that's the important issue, and we recognize it as such, then I could support the idea of merge doing this restore while other commands don't. Given what you've said here and thinking anew about merge and missing subtrees, I think the best approach might simply be what we currently do with files: Skip the missing subtree and record non-inheritable mergeinfo so the missing subtree is handled correctly (i.e. the mergeinfo reflects the fact that the merge never touched the missing subtree). This has a few things going for it: A) It's consistent with 1.5-1.6's treatment of missing files. B) As Daniel hinted at in http://svn.haxx.se/dev/archive-2010-08/0189.shtml, it's consistent with how merge tracking treats every other type of missing subtree (e.g. shallow WCs, switches, paths missing due to authz restrictions). C) It makes no assumptions about what the user intended, it just deals with the fact that the subtrees are missing. D) It allows the merge to complete, no errors mid-merge. E) It allows the missing subtrees to be restored via update (either before or after the merge is committed) and for the merge to be repeated. The repeat merge will notice that the merge never touched the subtrees and will drive the editor such that only those subtrees have the merge repeated. Any thoughts to this approach? Another plus: F) It means the merge algorithm has one less case that can choke it, which is better than relying on a check to have been done beforehand. It makes the subroutines easier to re-use (callable by GUIs etc.). And it could be extremely difficult to make sure that such an external check exactly matches the merge code in all the corner cases. The only minus I can think of: In many ways we would serve our users better by simply not allowing them to get into complex situations and instead just disallowing such a merge. But the many advantages seem to outweigh that, and your suggestion sounds close to perfect. If you can do that without much additional complexity, +1. I really thought that would be the case, as most of the logic is already in place to handle other types of missing subtrees. But after hacking on this entirely too long and finding new bugs at every turn, I had some
