Re: SIGPIPE handling

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Manfred Spraul <manfred(at)colorfullife(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: SIGPIPE handling
Date: 2003-11-17 15:47:21
Message-ID: 200311171547.hAHFlLE07302@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Manfred Spraul wrote:
> Bruce Momjian wrote:
>
> >Here is my logic --- 99% of apps don't install a SIGPIPE signal handler,
> >and 90% will not add a SIGPIPE/SIG_IGN call to their applications. I
> >guess I am looking for something that would allow the performance
> >benefit of not doing a pgsignal() call around very send() for the
> >majority of our apps. What was the speed improvement?
> >
> >
> Around 10% for a heavily multithreaded app on an 8-way Xeon server. Far
> less for a single threaded app and far less for uniprocessor systems:
> the kernel must update the pending queue of all threads and that causes
> lots of contention for the (per-process) spinlock that protects the
> signal handlers.

OK, I know you had a flag for pgbench, and that doesn't use threads.
What speedup do you see there?

> >Granted, we need to do something because our current setup isn't even
> >thread-safe. Also, how is your patch more thread-safe than the old one?
> >The detection is thread-safe, but I don't see how the use is.
> >
> First function in main():
>
> signal(SIGPIPE, SIG_IGN);
> PQsetsighandling(1);
>
> This results in perfectly thread-safe sigpipe handling. If it's a
> multithreaded app that needs correct correct per-thread delivery of
> SIGPIPE signals for console IO, then the libpq user must implement the
> sequence I describe below.

I would not expect a library to require me to do something in my code to
be thread-safe --- either it is or it isn't. I wonder if we should use
the sequence you list below when we compile using
--enable-thread-safety. We already use thread calls in port/thread.c,
specifically pthread_mutex_lock(). Why not make it work 100% if then
enable that build option?

> >This runs thread1 with SIGPIPE as SIG_DFL.
> >
> >
> Correct. A thread safe sequence might be something like:
>
> pthread_sigmask(SIG_BLOCK,{SIGPIPE});
> send();
> if (sigpending(SIGPIPE) {
> sigwait({SIGPIPE},);
> }
> pthread_sigmask(SIG_UNBLOCK,{SIGPIPE});
>
> But this sequence only works for users that link against libpthread. And
> the same sequence with sigprocmask is undefined for multithreaded apps.

Again, let's get it working perfect if they say they are going to use
threads with libpq. Does it work OK if the app doesn't use threading?

Does sigpending/sigwait work efficiently for threads? Another idea is
to go with a thread-local storage boolean for each thread, and check
that in a signal handler we install. Seems synchronous signals like
SIGPIPE are delivered to the thread that invoked them, and we can check
thread-local storage to see if we were in a send() loop at the time of
signal delivery.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Manfred Spraul 2003-11-17 17:59:55 Re: SIGPIPE handling
Previous Message Bruce Momjian 2003-11-17 15:33:49 Re: [PATCHES] SRA Win32 sync() code