Re: [PATCHES] Re: some patches

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  |
+----------------------------------------------------------------------+