Thread: sinval.c / sinvaladt.c restructuring

sinval.c / sinvaladt.c restructuring

From
Alvaro Herrera
Date:
I just modified the interactions in sinval.c and sinvaladt.c per
http://thread.gmane.org/gmane.comp.db.postgresql.devel.patches/18820/focus=18822

It looks a lot saner this way: the code that actually deals with the
queue, including locking etc, is all in sinvaladt.c.  This means that
the struct definition of the queue, and the queue pointer, are now
internal "implementation details" inside sinvaladt.c.

One side effect of this change is that the call to SendPostmasterSignal
now occurs after the lock has been released.  ISTM this is a good idea
on general principles (no syscalls in lwlocked code), but I'm wondering
if I created a thundering hoard problem that did not exist before.

All tests pass.

As a test of Review Board, I uploaded the patch to it:
http://reviewdemo.postgresql.org/r/19/

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Attachment

Re: sinval.c / sinvaladt.c restructuring

From
Bruce Momjian
Date:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Alvaro Herrera wrote:
> I just modified the interactions in sinval.c and sinvaladt.c per
> http://thread.gmane.org/gmane.comp.db.postgresql.devel.patches/18820/focus=18822
>
> It looks a lot saner this way: the code that actually deals with the
> queue, including locking etc, is all in sinvaladt.c.  This means that
> the struct definition of the queue, and the queue pointer, are now
> internal "implementation details" inside sinvaladt.c.
>
> One side effect of this change is that the call to SendPostmasterSignal
> now occurs after the lock has been released.  ISTM this is a good idea
> on general principles (no syscalls in lwlocked code), but I'm wondering
> if I created a thundering hoard problem that did not exist before.
>
> All tests pass.
>
> As a test of Review Board, I uploaded the patch to it:
> http://reviewdemo.postgresql.org/r/19/
>
> --
> Alvaro Herrera                                http://www.CommandPrompt.com/
> The PostgreSQL Company - Command Prompt, Inc.

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>        choose an index scan if your joining column's datatypes do not
>        match

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://postgres.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: sinval.c / sinvaladt.c restructuring

From
Tom Lane
Date:
Alvaro Herrera <alvherre@commandprompt.com> writes:
> One side effect of this change is that the call to SendPostmasterSignal
> now occurs after the lock has been released.  ISTM this is a good idea
> on general principles (no syscalls in lwlocked code), but I'm wondering
> if I created a thundering hoard problem that did not exist before.

Forgot to reply to this earlier, but I think this is OK.  The test for
setting signal_postmaster is for exact equality of numMsgs to a
threshold, so at least in simple cases only one backend will send the
signal.  You could imagine scenarios where the slowest backend is trying
to catch up and numMsgs oscillates around the threshold, but it seems
unlikely to be a problem in practice.  I concur that moving the signal
out of the locked code is a good thing.  (Maybe move the elog(DEBUG4)
as well?)

            regards, tom lane