> 
> Massimo, this is a fine set of patches.  I want to comment on a few.
> 
> > > async.patch
> > > 
> > > 	pathes for debugging and to reduce conflicts and locks between users
> > > 	of pg_listeners.
> > 
> > 	Appears to be massive changes since patch created...Not Applied.
> 
> That is a shame.  Sounds like a nice patch.  Massimo, I am curious about
> your comments on my changes to the lock source code.  Was it a help?
> 
> Hard to imagine what has changed since August 21st.

fixed now

> > > lock.patch
> > > 
> > > 	I have rewritten lock.c cleaning up the code and adding better
> > > 	assert checking I have also added some fields to the lock and 
> > > 	xid tags for better support of user locks. There is also a new
> > > 	function which returns an array of pids owning a lock.
> > > 	I'm using this code from over six months and it works fine.
> > 
> > 	Applied...
> 
> Again, any work on locks is good.  Did my changes help?

I'm not completely convinced of the new lock priority. I will study your
code and make some test. I suspect it may introduce some deadlocks in the
listen/notify code.

> > > ps-status.patch
> > > 
> > > 	macros for ps status, used by postgres.c and utility.c.
> > > 	Unfortunately ps status is system dependent and the current
> > > 	code doesn't work on linux. The use of macros confines system
> > > 	dependency to into one file (ps-status.h). Users of other
> > > 	operating systems should check this code and submit new macros.
> > 
> > 	Applied...but...
> > 
> > 	...is there a reason why we aren't using setproctitle() where
> > applicable, or is this what you mean with the 'submit new macros' comment?
> > Or is this totally unrelated? :)
> 
> I think so.  Someone with setproctitle needs to give us code that works
> on their machine.  We can then add a configure check so OS's with the
> feature can use it.  One issue is whether we want the function called
> for each backend query because it may be too large a performance hit. 
> No idea how to deal with this.

ps status is optional. If we can't do it on some OS we simply define empty
macros and all is ok. We must however be careful with side effects inside
ps macros in utility.c.

> > > sinval.patch
> > > 
> > > 	fixes a problem in SI cache which causes table overflow if some
> > > 	backend is idle for a long time while other backends keep adding
> > > 	entries.
> > > 	It uses the new signal handling implemented in tprintf.patch.
> > > 	I have also increacasesed the max number of backends from 32 to 64
> > > 	and the table size from 1000 to 5000.
> > > 	I don't know if anybody is working on SI, but until another
> > > 	solution is found this patch fixes the problem. I have received
> > > 	messages from other people reporting the same problem which I
> > > 	fixed many months ago.
> > 
> > 	Applied...
> 
> I was hoping you would submit this one.  Sounds like a needed patch, and
> something few of use know how to debug.

Is Vadim working on shared cache? Maybe this patch could become obsolete
in the next version. But for now it is necessary. I had very nasty problems
with this.

> > > socket-flock.patch
> > > 
> > > 	use advisory locks to check if the unix socket can be deleted.
> > > 	A running postmaster keeps a lock on that file. A starting
> > > 	postmaster exits if the file exists and is locked, otherwise
> > > 	it deletes the sockets and proceeds.
> > > 	This avoid the need to remove manually the file after a postmaster
> > > 	or system crash.
> > > 	I don't know if flock is available on any system. If not we could
> > > 	define a HAVE_FLOCK set by configure.
> > 
> > 	Definitely applied...
> 
> This is certainly a great idea.  Deleting that lock file is a pain.
> 
> I am almost certain we are going to need a configure check on this one.
> flock/lockf() are supported on almost all systems, but usually not both.
> Perhaps someone with lockf and not flock can give us some code.

Yes, this must be checked on every OS. I have only linux to test.

> > > tprintf.patch
> > > 
> > >  	tprintf.patch
> > > 
> > > 	adds functions and macros which implement a conditional trace package
> > > 	with the ability to change flags and numeric options of running
> > > 	backends at runtime.
> > > 	Options/flags can be specified in the command line and/or read from
> > > 	the file pg_options in the data directory.
> > 
> > 	Applied....
> > 
> > 	Just curious, but any documentation forthcoming on some of this
> > stuff?  pg_options and the assert stuff are the one that jump to mind...
> 
> Good idea.  If you can give me a comment about it, I can put something
> in the FAQ under debugging.  Sounds like we need manual page update, if
> that was not already in the patch.

I will write some documentation. Any comments on the new signal handling ?

I would like also to discuss on the different ways used to print debug
messages. At the moment we have stdout, stderr, tprintf with syslog and
elog. Maybe we could establish some standard ways to print messages.
And we could put all the various debug flags and options in pg_options.

I have also another idea. When the postmaster is started it should delete
all tuples left in pg_listener because all pids refer to processes now dead.
I'm thinking of doing an ftruncate on all data/*/pg_listener at postmaster
startup. This should work because there aren't any indices on that table.
I'm doing this in the shell script which starts postgres and it semms to
work fine. Any comments ?

-- 
Massimo Dal Zotto

+----------------------------------------------------------------------+
|  Massimo Dal Zotto                email:  dz@cs.unitn.it             |
|  Via Marconi, 141                 phone:  ++39-461-534251            |
|  38057 Pergine Valsugana (TN)     www:  http://www.cs.unitn.it/~dz/  |
|  Italy                            pgp:  finger dz@tango.cs.unitn.it  |
+----------------------------------------------------------------------+

Thu, 27 Aug 1998 00:49:11 +0200 (MET DST)
Massimo Dal Zotto <dz@cs.unitn.it>
Re: [PATCHES] Re: some patches
> > > > 	xid tags for better support of user locks. There is also a new
> > > > 	function which returns an array of pids owning a lock.
> > > > 	I'm using this code from over six months and it works fine.
> > > 
> > > 	Applied...
> > 
> > Again, any work on locks is good.  Did my changes help?
> 
> I'm not completely convinced of the new lock priority. I will study your
> code and make some test. I suspect it may introduce some deadlocks in the
> listen/notify code.

Very possible.  listen/notify has a different locking priority.

> 
> > > > ps-status.patch
> > > > 
> > > > 	macros for ps status, used by postgres.c and utility.c.
> > > > 	Unfortunately ps status is system dependent and the current
> > > > 	code doesn't work on linux. The use of macros confines system
> > > > 	dependency to into one file (ps-status.h). Users of other
> > > > 	operating systems should check this code and submit new macros.
> > > 
> > > 	Applied...but...
> > > 
> > > 	...is there a reason why we aren't using setproctitle() where
> > > applicable, or is this what you mean with the 'submit new macros' comment?
> > > Or is this totally unrelated? :)
> > 
> > I think so.  Someone with setproctitle needs to give us code that works
> > on their machine.  We can then add a configure check so OS's with the
> > feature can use it.  One issue is whether we want the function called
> > for each backend query because it may be too large a performance hit. 
> > No idea how to deal with this.
> 
> ps status is optional. If we can't do it on some OS we simply define empty
> macros and all is ok. We must however be careful with side effects inside
> ps macros in utility.c.

Yes, good idea to move it out as a macro.  It was all over the code.

> > > > 	and the table size from 1000 to 5000.
> > > > 	I don't know if anybody is working on SI, but until another
> > > > 	solution is found this patch fixes the problem. I have received
> > > > 	messages from other people reporting the same problem which I
> > > > 	fixed many months ago.
> > > 
> > > 	Applied...
> > 
> > I was hoping you would submit this one.  Sounds like a needed patch, and
> > something few of use know how to debug.
> 
> Is Vadim working on shared cache? Maybe this patch could become obsolete
> in the next version. But for now it is necessary. I had very nasty problems
> with this.

I haven't heard him talking about it recently.

> 
> > > > socket-flock.patch
> > > > 
> > > > 	use advisory locks to check if the unix socket can be deleted.
> > > > 	A running postmaster keeps a lock on that file. A starting
> > > > 	postmaster exits if the file exists and is locked, otherwise
> > > > 	it deletes the sockets and proceeds.
> > > > 	This avoid the need to remove manually the file after a postmaster
> > > > 	or system crash.
> > > > 	I don't know if flock is available on any system. If not we could
> > > > 	define a HAVE_FLOCK set by configure.
> > > 
> > > 	Definitely applied...
> > 
> > This is certainly a great idea.  Deleting that lock file is a pain.
> > 
> > I am almost certain we are going to need a configure check on this one.
> > flock/lockf() are supported on almost all systems, but usually not both.
> > Perhaps someone with lockf and not flock can give us some code.
> 
> Yes, this must be checked on every OS. I have only linux to test.

Yep.


> > Good idea.  If you can give me a comment about it, I can put something
> > in the FAQ under debugging.  Sounds like we need manual page update, if
> > that was not already in the patch.
> 
> I will write some documentation. Any comments on the new signal handling ?

I did not notice that.  What new signal handling?

> 
> I would like also to discuss on the different ways used to print debug
> messages. At the moment we have stdout, stderr, tprintf with syslog and
> elog. Maybe we could establish some standard ways to print messages.
> And we could put all the various debug flags and options in pg_options.

Yes.  We have talked about syslog for ages, but no one has done it.  It
is on the TODO list.  Seems like unifying everything to use syslog is
the most powerful.

> 
> I have also another idea. When the postmaster is started it should delete
> all tuples left in pg_listener because all pids refer to processes now dead.
> I'm thinking of doing an ftruncate on all data/*/pg_listener at postmaster
> startup. This should work because there aren't any indices on that table.
> I'm doing this in the shell script which starts postgres and it semms to
> work fine. Any comments ?


Yes, they should be deleted.  We should probably delete all the old temp/sort
files too.  I think you can just delete the table, as long as no backend
is running.  There really is no information kept between postmaster
startups that would get out-of-sync if a table had zero length.  Try it
and if it works, go for it.

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

Sat, 29 Aug 1998 00:15:28 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [PATCHES] Re: some patches