On Tue, 2011-05-10 at 05:29 -0400, Greg Stein wrote:
> On Mon, May 9, 2011 at 18:30, Julian Foad <julian.foad_at_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
Received on 2011-05-10 12:46:04 CEST