bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: do_verify patch for cvs top-of-tree


From: Mark D. Baushke
Subject: Re: do_verify patch for cvs top-of-tree
Date: Tue, 21 Aug 2001 22:41:01 -0700

On Mon, 20 Aug 2001 12:57:31 -0400, "Derek R. Price" wrote:
>
> "Mark D. Baushke" wrote:
>
> > Looking at portability, I see a problem with satifying the first of
> > your requests.
> >
> > > 1. This looks pretty good on cursory inspection, but the logmsg file
> > > shouldn't always be reloaded after the verifymsg script runs.  The
> > > timestamp of the file should be cached and checked for changes before
> > > reloading.
> >
> > Unless I use sleep_past(), to wait until the next second between
> > writing out the log message file for the verification script, or I use
> > unportable sub-second entries from the stat structure, there is a race
> > condition for determining if the log message needs to be reloaded or
> > not. To be honest, I would rather re-read the file all the time
> > instead of adding an extra second to the commit...
> >
> > Here is the patch that makes it use sleep_past(). What are your
> > thoughts on this matter?
>
> Well, I don't think the extra second would bother any but the most
> ineptly scripted commits much, considering that this check will only
> happen once and will usually be immediately following a user edit,
> which takes forever relative to the second.  Besides - it's not
> always a full second.

I was under the impression that it happens once per directory being
changed. If you have many hundreds of directories, it can really add
up to some significant time that you are doing a sleep_past(). Ten
directories can end up being ten seconds of waiting which is not too
bad, but for big projects with hundreds of directories getting updated
as for a merge, I was under the impression that sleep_past() could
keep the locks on the repository a lot longer that is really desirable
for very little gain in speed over just reading the file again which
should take much less elappsed time than sleep_past() is waiting.

Of course, on a really old system with slow disk, maybe 'stat' is the
way to go... we use hardware raid5 systems for our cvs servers, so
it is not a big deal for us to just read the file all the time.

> Hmm.  I'm still inclined to think that the single stat is much less
> CPU and IO intensive when no changes have been made.  I've seen some
> pretty drasticaly overloaded systems which make this scenario
> preferrable - CVS can be quite a resource hog as it stands.

I suppose that depends on the filesystem being used. Most log messages
are very small and would easily fit in a read buffer. I understand
that reading a small file is almost as cheap as a stat() on some
systems.

> Of course on a system where you know the script will always change
> the log message, the stat is sub-optimal.

Yes, or on a system where you have many hundreds of directories
potentially changing at the same time with the same log message, the
sleep_past() after the first CVS_STAT() is also sub-optimal.

> What about a CVSROOT/config option?

Okay, that is not hard to implement. If it helps to end the debate and
get the patch installed, then I'm all for it.

> RereadLogAfterVerify=no|stat|always or some such?

Okay. I have implemented it. Values are:

  RereadLogAfterVerify=no|never|stat|always|yes

The default is RereadLogAfterVerify=never to retain the status quo.

See the patch attached to http://ccvs.cvshome.org/issues/show_bug.cgi?id=19

I added some more documentation for the new option and enhanced the
example verification script to do something to modify the log message.

I didn't go into the reasons that a user may want to choose 'always'
vs 'stat' as I thought I should have you double-check my understanding
of how often the verifymsg script is being run. Let me know if you
think there needs to be another paragraph on that issue or not.

> Any differeing opinions?  The history of the issue is here:
> http://ccvs.cvshome.org/issues/show_bug.cgi?id=19 and I attached
> Mark's new patch to it rather than forward it.

I did the same.

        Thanks,
        -- Mark



reply via email to

[Prev in Thread] Current Thread [Next in Thread]