On Tue, 2011-05-10 at 02:12 +0200, Bert Huijben wrote: > > > -----Original Message----- > > From: Mark Phippard [mailto:markp...@gmail.com] > > Sent: dinsdag 10 mei 2011 1:16 > > To: Julian Foad > > Cc: dev@subversion.apache.org > > Subject: Re: [PATCH] WC DB verification 1 > > > > Could we tie it into cleanup? Then other tools like TortoiseSVN, > > Subclipse, AnkhSVN could invoke it too. > > > > > > On Mon, May 9, 2011 at 6:30 PM, Julian Foad <julian.f...@wandisco.com> > > wrote: > > > I'm planning to commit the attached wc-db-verification-1.patch, subject > > > to any advice on how to best fit it in to the code base or other > > > concerns, in order to get a DB "self-check" function started. > > > > > > I think we need something like this. Earlier today I found that "svn > > > status" showed I had a clean, single-rev WC, while "svnversion" said it > > > was mixed-rev and switched. Investigation showed there were orphaned > > > base node rows in the DB which weren't seen by "svn st" but were seen by > > > "svnversion". I'm not interested in how that particular state came to > > > be, as I've run hundreds of buggy trunk builds on this WC over many > > > months. What I do want is to be able to run a set of checks on a DB > > > that detects basic rule violations like that. > > > > > > Decisions about when and how to run it can come later. Of course if we > > > plan to run it frequently and automatically that would mean we'd want to > > > make sure it only did fast checks and is efficiently coded whereas if it > > > remains a manual intervention for devs then that's no concern. Thoughts > > > about this are welcome too. > > I would suggest adding a new function for this purpose; and maybe a separate > program to call it for our beta cycle.
I'm happy to make a separate program to call it if that's what we prefer. > This code should never be necessary once we call the db stable. The idea is similar to validating postconditions whenever we modify the DB. It is "assertions" but at a higher level than ones we would put in-line in the code. It will help to make the DB stable. It should be called only during testing and debugging, but should remain available during testing and debugging forever. > And making > the normal cleanup very slow makes it less useful for its normal task. Sure. > I'm not sure about what you are testing in this initial patch though: The > only functions that can insert rows in NODES always use > svn_relpath_dirname() for creating parent_relpath, so if you find databases > where that set incorrectly you can be 100% certain those databases that are > hand edited. It's verifying conditions that "must be true". It should catch any case where a DB has been hand-edited or where we have accidentally added some kind of row-insertion code that is broken. > The parent exists checks are more useful, but should also look at op-depth, > etc. to really find problems. (The op-depth root must always exist and every > intermediate too) Yes, this is exactly the kind of useful extensions that I want us to make. - Julian
