Re: [COMMITTERS] pgsql: Move btbulkdelete's vacuum_delay_point()

2006-02-15 Thread Simon Riggs 
On Tue, 2006-02-14 at 17:13 -0500, Tom Lane wrote:
> Simon Riggs <[EMAIL PROTECTED]> writes:
> > Perhaps if vacuum_delay_point() contained a timer check, we'd be able to
> > see if any gap between vacuum delays was more than the actual delay
> > itself. It would be nice to know they are all gone, forever.
> 
> vacuum_delay_point is intended to be cheap enough (in the non-delay
> case) that no one would have any hesitation about dropping it into
> loops.  With a timer check in there, that might not be true, so I'm
> resistant to doing it unconditionally.  But I could see having some
> #ifdef'd code that could be conditionally compiled in to measure the
> maximum inter-delay-point time in a development build.

I'm thinking something like this might work better, since this is the
issue I/we really care about:

   if (LWLockNumHeldByMe() == 0)
pg_usleep(msec * 1000L);
   else
elog(WARNING, "losing sleep because internal locks are held");

or is it not worth losing sleep over? ;-)

The additional code is only executed *if* we are going to sleep, so a
few extra cycles won't hurt when we are just about to sleep for at least
10 milliseconds.

We could make similar checks at most of the other sleep locations,
modifying the number of held locks appropriately.

Best Regards, Simon Riggs

Index: src/backend/commands/vacuum.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.324
diff -c -r1.324 vacuum.c
*** src/backend/commands/vacuum.c	11 Feb 2006 23:31:33 -	1.324
--- src/backend/commands/vacuum.c	15 Feb 2006 11:00:13 -
***
*** 37,42 
--- 37,43 
  #include "miscadmin.h"
  #include "postmaster/autovacuum.h"
  #include "storage/freespace.h"
+ #include "storage/lwlock.h"
  #include "storage/procarray.h"
  #include "storage/smgr.h"
  #include "tcop/pquery.h"
***
*** 3441,3447 
  		if (msec > VacuumCostDelay * 4)
  			msec = VacuumCostDelay * 4;
  
! 		pg_usleep(msec * 1000L);
  
  		VacuumCostBalance = 0;
  
--- 3442,3451 
  		if (msec > VacuumCostDelay * 4)
  			msec = VacuumCostDelay * 4;
  
! if (LWLockNumHeldByMe() == 0)
! 		pg_usleep(msec * 1000L);
! else
! 			elog(WARNING, "losing sleep because internal locks are held");
  
  		VacuumCostBalance = 0;
  
Index: src/backend/storage/lmgr/lwlock.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/storage/lmgr/lwlock.c,v
retrieving revision 1.37
diff -c -r1.37 lwlock.c
*** src/backend/storage/lmgr/lwlock.c	29 Dec 2005 18:08:05 -	1.37
--- src/backend/storage/lmgr/lwlock.c	15 Feb 2006 11:00:14 -
***
*** 604,606 
--- 604,612 
  	}
  	return false;
  }
+ 
+ int
+ LWLockNumHeldByMe(void)
+ {
+ return num_held_lwlocks;
+ }
Index: src/include/storage/lwlock.h
===
RCS file: /projects/cvsroot/pgsql/src/include/storage/lwlock.h,v
retrieving revision 1.26
diff -c -r1.26 lwlock.h
*** src/include/storage/lwlock.h	19 Jan 2006 04:45:38 -	1.26
--- src/include/storage/lwlock.h	15 Feb 2006 11:00:18 -
***
*** 70,75 
--- 70,76 
  extern void LWLockRelease(LWLockId lockid);
  extern void LWLockReleaseAll(void);
  extern bool LWLockHeldByMe(LWLockId lockid);
+ extern int  LWLockNumHeldByMe(void);
  
  extern int	NumLWLocks(void);
  extern Size LWLockShmemSize(void);

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [COMMITTERS] pgsql: Move btbulkdelete's vacuum_delay_point()

2006-02-15 Thread Tom Lane 
Simon Riggs <[EMAIL PROTECTED]> writes:
>if (LWLockNumHeldByMe() == 0)
>   pg_usleep(msec * 1000L);
>else
> elog(WARNING, "losing sleep because internal locks are held");

I did some testing yesterday with an Assert added to vacuum_delay_point:
Assert(InterruptHoldoffCount == 0 && CritSectionCount == 0);
which is basically asserting that ProcessInterrupts is allowed to
execute an interrupt.  I desisted from committing this test, though,
because it's not clear that callers should not be allowed to call
vacuum_delay_point in such cases --- for the current callers it proved
fairly easy to locate the calls at places where this is certain to be OK,
but I don't want to wire in a requirement of it.

In any case, such tests don't help for the question of whether we've
missed any major loops that ought to contain vacuum_delay_point calls.
(If we were to add a worst-case-interval measurement, it might be
reasonable to not count calls that don't meet the ProcessInterrupts
condition.)

regards, tom lane

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

[COMMITTERS] pgsql: Since we only use libld on AIX, don't include it in LIBS on any

2006-02-15 Thread Tom Lane 
Log Message:
---
Since we only use libld on AIX, don't include it in LIBS on any other
platforms (it does exist on HPUX, for one).  We could probably even make
this a test for specific AIX versions, but I don't know which ones need it.

Modified Files:
--
pgsql:
configure.in (r1.448 -> r1.449)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/configure.in.diff?r1=1.448&r2=1.449)
configure (r1.478 -> r1.479)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/configure.diff?r1=1.478&r2=1.479)

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

[COMMITTERS] pgsql: Put the first section of a chapter in its own chunk.

2006-02-15 Thread Peter Eisentraut 
Log Message:
---
Put the first section of a chapter in its own chunk.

Modified Files:
--
pgsql/doc/src/sgml:
stylesheet.dsl (r1.29 -> r1.30)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/stylesheet.dsl.diff?r1=1.29&r2=1.30)

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq