On Tue, 2011-05-10 at 05:29 -0400, Greg Stein wrote: > On Mon, May 9, 2011 at 18:30, 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.
> Regarding the patch itself: Thanks for this good feedback Greg. > * I dislike creating yet another wc_db_foo file. Just put the damned > thing into wc_db_util.c. These files are not so big that we have to > break them up. wc_db.c, sure. But let's not keep adding files that are > just dozens of lines. It makes it hard to answer the question, "where > is that function?" I agree there's merit in not splitting things up too much. I'll note however that wc_db_util is documented as being lower-level helpers for use by wc_db, whereas this is more like a test driver for wc_db. Not sure that's a really good place but I can put it there for now. > * There is a rough sketch of wc-checks.sql. Is there something that > can be done where, for developers, more intense checks are performed? > And even better, can some of these checks be expressed as triggers, in > order to trap problems immediately? That looks interesting and good. I'll certainly try to expand the use of triggers for on-the-fly validation. We can of course have the option of disabling it when we're confident in correctness and want more speed. > * db_verify() should at least follow precedent with a > VERIFY_USABLE_WCROOT() call. (why'd you omit that?) Oversight. > * the (new) typical pattern is to pass a WCROOT rather than the > individual SDB and WC_ID, so _verify_nodes should have its args > changed Will look/do. > * wtf is relpath_depth duplicated into the verify file? serious badness. Just hacking. Will fix. > * there is no such comment as "TODO:" ... we use "###" When I use a comment it means exactly what I want it to mean. :-P Thanks and I'll hack on this on the train I'm about to catch. - Julian
