Skip site navigation (1) Skip section navigation (2)

Re: Listen / Notify - what to do when the queue is full

From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: Joachim Wieland <joe(at)mcknight(dot)de>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Greg Smith <greg(at)2ndquadrant(dot)com>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, "Florian G(dot) Pflug" <fgp(at)phlo(dot)org>, Josh Berkus <josh(at)agliodbs(dot)com>, pgsql-hackers(at)postgresql(dot)org, Arnaud Betremieux <arnaud(dot)betremieux(at)keyconsulting(dot)fr>
Subject: Re: Listen / Notify - what to do when the queue is full
Date: 2010-02-14 13:46:23
Message-ID: 1266155183.7341.9124.camel@ebony (view raw or flat)
Thread:
Lists: pgsql-hackers
On Thu, 2010-02-11 at 00:52 +0100, Joachim Wieland wrote:
> On Mon, Feb 8, 2010 at 5:16 PM, Alvaro Herrera
> <alvherre(at)commandprompt(dot)com> wrote:
> > These are the on-disk notifications, right?  It seems to me a bit
> > wasteful to store channel name always as NAMEDATALEN bytes.  Can we
> > truncate it at its strlen?
> 
> Attached is a new and hopefully more or less final patch for LISTEN / NOTIFY.
> 
> The following items have been addressed in this patch:
> 
>  - only store strlen(channel) instead of NAMEDATALEN bytes on disk
>  - limit to 7-bit ASCII
>  - forbid 2PC and LISTEN/NOTIFY for now
>  - documentation changes
>  - add missing tab completion for NOTIFY
>  - fix pg_notify() behavior with respect to NULLs, too long and too
> short parameters
>  - rebased to current HEAD, OID conflicts resolved


Some minor review comments without having taken in much of previous
discussion:

* Patch doesn't apply cleanly anymore, close.

* In async.c you say "new async notification model". Please say "async
notification model in 9.0 is". In (2) you say there is a central queue,
but don't describe purpose of queue or what it contains. Some other
typos in header comments.

* There is no mention of what to do with pg_notify at checkpoint. Look
at how pg_subtrans handles this. Should pg_notify do the same?

* Is there a lazy backend avoidance scheme as exists for relcache
invalidation messages? see storage/ipc/sinval...
OK, I see asyncQueueFillWarning() but nowhere else says it exists and
there aren't any comments in it to say what it does or why

* What do you expect the DBA to do when they receive a queue fill
warning? Where is that written down?

* Not clear of the purpose of backendSendsNotifications. In
AtCommit_NotifyAfterCommit() the logic seems strange. Code is
 	/* Allow transactions that have not executed
		 LISTEN/UNLISTEN/NOTIFY to
 	 * return as soon as possible */
 	if (!pendingActions && !backendSendsNotifications)
 		return;
but since backendSendsNotifications is set true earlier there is no fast
path. 

* AtSubCommit_Notify doesn't seem to have a fastpath when no notify
commands have been executed.

* In Send_Notify() you throw ERROR while still holding the lock. It
seems better practice to drop the lock then ERROR.

* Why is Send_Notify() a separate function? It's only called from one
point in the code.

* We know that sometimes a FATAL error can occur without properly
processing the abort. Do we depend upon this never happening?

* Can we make NUM_ASYNC_BUFFERS = 8? 4 just seems too small

* backendSendsNotifications is future tense. The variable should be
called something like hasSentNotifications. Couple of other variables
with similar names

Hope that helps

-- 
 Simon Riggs           www.2ndQuadrant.com


In response to

Responses

pgsql-hackers by date

Next:From: Greg StarkDate: 2010-02-14 14:03:44
Subject: Re: Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)
Previous:From: Simon RiggsDate: 2010-02-14 05:29:45
Subject: Re: idle in txn query cancellation

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group