Re: [PATCHES] Re: some patches - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: [PATCHES] Re: some patches
Date
Msg-id 199808290415.AAA29249@candle.pha.pa.us
Whole thread Raw
In response to Re: [PATCHES] Re: some patches  (Massimo Dal Zotto <dz@cs.unitn.it>)
List pgsql-hackers
> > > >     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)

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Segmentation fault with lo_export
Next
From: Bruce Momjian
Date:
Subject: Re: PostgreSQL under BSD/OS