Listen / Notify rewrite
Hi,
Attached is a patch for a new listen/notify implementation.
In a few words, the patch reimplements listen/notify as an slru-based queue
which works similar to the sinval structure. Essentially it is a ring buffer on
disk with pages mapped into shared memory for read/write access.
Additionally the patch does the following (see below for details):
1. It removes the pg_listener relation and
2. adds the possibility to specify a payload parameter, i.e. executing in SQL
"NOTIFY foo 'payload';" and 'payload' will be delivered to any listening
backend.
3. Every distinct notification is delivered.
4. Order is preserved, i.e. if txn 1 first does NOTIFY foo, then NOTIFY bar, a
backend (listening to both "foo" and "bar") will always first receive the
notification "foo" and then the notification "bar".
5. It's now "listen to a channel", not "listen to a relation" anymore...
Details:
1. Instead of placing the queue into shared memory only I propose to create a
new subdirectory pg_notify/ and make the queue slru-based, such that we do not
risk blocking. Several people here have pointed out that blocking is a true
no-go for a new listen/notify implementation. With an slru-based queue we have
so much space that blocking is really unlikely even in periods with extreme
notify bursts.
Regarding performance, the slru-queue is not fsync-ed to disk so most activity
would be in the OS file cache memory anyway and most backends will probably
work on the same pages most of the time. However more locking overhead is
required in comparison to a shared-memory-only implementation.
There is one doubt that I have: Currently the patch adds notifications to the
queue after the transaction has committed to clog. The advantage is that we do
not need to take care of visibility. When we add notifications to the queue, we
have committed our transaction already and all reading backends are not in a
transaction anyway, so everything is visible to everyone and we can just write
to and read from the queue.
However, if for some reason we cannot write to the slru files in the pg_notify/
directory we might want to roll back the current transaction but with the
proposed patch we cannot because we have already committed...
But... if there is a problem with the pg_notify/ directory, then something is
fundamentally wrong on the file system of the database server and pg_subtrans/
and pg_clog/ are probably affected by the same problem... One possible solution
would be to write to the queue before committing and adding the TransactionID.
Then other backends can check if our TransactionID has successfully committed
or not. Not sure if this is worth the overhead however...
2. The payload parameter is optional. A notifying client can either call
"NOTIFY foo;" or "NOTIFY foo 'payload';". The length of the payload is
currently limited to 128 characters... Not sure if we should allow longer
payload strings... If there is more complex data to be transferred, the sending
transaction can always put all of that data into a relation and just send the
id of the entry. If no payload is specified, then this is treated internally
like an empty payload. Consequently an empty string will be delivered as the
payload to the listening backend.
3. Not every notification is delivered, only distinct notifications (per
transaction).
In other words, each sending transaction eliminates duplicate notifications
that have the exact same payload and channel name as another notification that
is already in the queue to be sent out.
Should we have an upper limit on the number of notifications that a transaction
is allowed to send? Without an upper limit, a client can open a transaction and
send a series of NOTIFYs, each with a different payload until its backend runs
out of memory...
4.
Sending Transaction does: This will be delivered (on commit):
NOTIFY foo; NOTIFY foo;
NOTIFY foo; NOTIFY bar 'value1';
NOTIFY bar 'value1'; NOTIFY bar 'value2';
NOTIFY foo;
NOTIFY bar 'value2';
NOTIFY bar 'value1';
Note that we do _not_ guarantee that notifications from transaction 1 are
always delivered before the notifications of transaction 2 just because
transaction 1 committed before transaction 2.
Let me know what you think,
Regards,
Joachim
Attachments:
listennotify.1.difftext/x-diff; charset=US-ASCII; name=listennotify.1.diffDownload+1193-938
On Nov 11, 2009, at 4:25 PM, Joachim Wieland wrote:
Hi,
Attached is a patch for a new listen/notify implementation.
In a few words, the patch reimplements listen/notify as an slru-
based queue
which works similar to the sinval structure. Essentially it is a
ring buffer on
disk with pages mapped into shared memory for read/write access.Additionally the patch does the following (see below for details):
1. It removes the pg_listener relation and
2. adds the possibility to specify a payload parameter, i.e.
executing in SQL
"NOTIFY foo 'payload';" and 'payload' will be delivered to any
listening
backend.
3. Every distinct notification is delivered.
4. Order is preserved, i.e. if txn 1 first does NOTIFY foo, then
NOTIFY bar, a
backend (listening to both "foo" and "bar") will always first
receive the
notification "foo" and then the notification "bar".
5. It's now "listen to a channel", not "listen to a relation"
anymore...
Hi Joachim,
Thank you for implementing this- LISTEN/NOTIFY without a payload has
been a major problem to work around for me.
I understand that coalescing multiple notifications of the same name
happens now, but I never understood why. I had hoped that someone
revisiting this "feature" would remove it. My use case is autonomous
transactions.
From a developer's standpoint, NOTIFY specifies a form of remote
trigger for further action- outside the transaction- to occur. From
that point of view, I don't see why NOTIFY foo; NOTIFY foo; is
equivalent to NOTIFY foo;. I understand the use case where a per-row
trigger could generate lots of spurious notifications, but it seems
that if anything is generating spurious notifications, the
notification is in the wrong place.
The documentation makes the strong implication that notification names
are usually table names, but with the new payload, this becomes even
less the case.
""I changed this table, take a look at it to see what's new". But no
such association is enforced by the NOTIFY and LISTEN commands." http://www.postgresql.org/docs/8.4/static/sql-notify.html
With the coalescing, I feel like I am being punished for using NOTIFY
with something other than a table name use case.
At least with this new payload, I can set the payload to the
transaction ID and be certain that all the notifications I sent are
processed (and in order even!) but could you explain why the
coalescing is still necessary?
While coalescing could be achieved on the receiving-end NOTIFY
callback ( if(payload ID was already processed) continue; ), non-
coalescing behavior cannot be achieved when the backend does the
coalescing.
For backwards compatibility, payload-less NOTIFY could coalesce while
NOTIFYs with a payload would not coalesce, but, on the other hand,
that might be confusing.
In any case, thank you for improving this long-neglected subsystem!
Best regards,
M
On Wed, Nov 11, 2009 at 10:25:05PM +0100, Joachim Wieland wrote:
Hi,
Attached is a patch for a new listen/notify implementation.
In a few words, the patch reimplements listen/notify as an slru-based queue
which works similar to the sinval structure. Essentially it is a ring buffer on
disk with pages mapped into shared memory for read/write access.
While I can't really comment on the implementation, from your
description it looks like a big improvement.
Nice work!
Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/
Show quoted text
Please line up in a tree and maintain the heap invariant while
boarding. Thank you for flying nlogn airlines.
2. adds the possibility to specify a payload parameter, i.e. executing
in SQL
"NOTIFY foo 'payload';" and 'payload' will be delivered to any
listening
backend.Thank you for implementing this- LISTEN/NOTIFY without a payload has
been a major problem to work around for me.
+1
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
/*
+ * This function is executed for every notification found in the queue in order
+ * to check if the current backend is listening on that channel. Not sure if we
+ * should further optimize this, for example convert to a sorted array and
+ * allow binary search on it...
+ */
+ static bool
+ IsListeningOn(const char *channel)I think a bsearch would be needed. Very busy servers that make heavy use of
notifies would be quite a penalty.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
On Thu, Nov 12, 2009 at 1:04 AM, Andrew Chernow <ac@esilo.com> wrote:
I think a bsearch would be needed. Very busy servers that make heavy use of
notifies would be quite a penalty.
In such an environment, how many relations/channels is a backend
typically listening to?
Do you have average / maximal numbers?
Regards,
Joachim
Andrew Chernow <ac@esilo.com> writes:
+ * This function is executed for every notification found in the queue in order + * to check if the current backend is listening on that channel. Not sure if we + * should further optimize this, for example convert to a sorted array and + * allow binary search on it...
I think a bsearch would be needed. Very busy servers that make heavy use of
notifies would be quite a penalty.
Premature optimization is the root of all evil ;-). Unless you've done
some profiling and can show that this is a hot spot, making it more
complicated isn't the thing to be doing now.
regards, tom lane
Joachim Wieland wrote:
On Thu, Nov 12, 2009 at 1:04 AM, Andrew Chernow <ac@esilo.com> wrote:
I think a bsearch would be needed. Very busy servers that make heavy use of
notifies would be quite a penalty.In such an environment, how many relations/channels is a backend
typically listening to?
Do you have average / maximal numbers?
We have a system where many libpq clients, ~2000 - 4000 per server (depends on
hardware), maintain a persistent backend connection. Each connection listens
for notifies, LISTEN 'client_xxx'. There are maybe 10 different reasons why a
NOTIFY 'client_xxx' is fired. Sometimes, notifies are broadcasted to all client
connections, or just portions of them.
The goal is real-time messaging to a large groups of computers/devices. Past
4000, the problem is distributed to a second, third, etc... server.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
"Martijn" == Martijn van Oosterhout <kleptog@svana.org> writes:
Hi,
Attached is a patch for a new listen/notify implementation.
In a few words, the patch reimplements listen/notify as an
slru-based queue which works similar to the sinval
structure. Essentially it is a ring buffer on disk with pages
mapped into shared memory for read/write access.
Martijn> While I can't really comment on the implementation, from your
Martijn> description it looks like a big improvement.
Does it cope with the case where a trigger is doing NOTIFY, and you do
a whole-table update, therefore dumping potentially millions of
notifications in at once?
(for example a rare maintenance operation on a table which has a
listen/notify arrangement triggered by single inserts or updates)
The existing implementation copes with that just fine.
--
Andrew (irc:RhodiumToad)
Premature optimization is the root of all evil ;-). Unless you've done
some profiling and can show that this is a hot spot, making it more
complicated isn't the thing to be doing now.
I'm thinking of how our system uses/abuses notifies, and began wondering
if several thousand backends listening with a large queue would perform
decently behind a linear search. At this point, I have no data either
way; only an assumption based off being burnt by sequential scans in the
past ;)
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
On Wed, Nov 11, 2009 at 5:48 PM, A.M. <agentm@themactionfaction.com> wrote:
At least with this new payload, I can set the payload to the transaction ID
and be certain that all the notifications I sent are processed (and in order
even!) but could you explain why the coalescing is still necessary?
Christmas comes early this year! :-).
three reasons:
*) it works that way now...a lot of people use this feature for all
kinds of subtle things and the behavior chould change as little as
possible
*) legacy issues aside, I think it's generally better behavior (how
many times do you need to be tapped on the shoulder?)
*) since you can trivially differentiate it (using xid, sequence,
etc), what's the fuss?
merlin
On Nov 11, 2009, at 9:28 PM, Merlin Moncure wrote:
On Wed, Nov 11, 2009 at 5:48 PM, A.M.
<agentm@themactionfaction.com> wrote:At least with this new payload, I can set the payload to the
transaction ID
and be certain that all the notifications I sent are processed
(and in order
even!) but could you explain why the coalescing is still necessary?Christmas comes early this year! :-).
three reasons:
*) it works that way now...a lot of people use this feature for all
kinds of subtle things and the behavior chould change as little as
possible
*) legacy issues aside, I think it's generally better behavior (how
many times do you need to be tapped on the shoulder?)
*) since you can trivially differentiate it (using xid, sequence,
etc), what's the fuss?
Except for the fact that the number of times a notification occurred
may be valuable information.
I thought of a compromise: add the number of times a notification was
generated (coalesced count+1) to the callback data. That would
satisfy any backwards compatibility concerns and my use case too!
Cheers,
M
Joachim Wieland <joe@mcknight.de> writes:
However, if for some reason we cannot write to the slru files in the pg_notify/
directory we might want to roll back the current transaction but with the
proposed patch we cannot because we have already committed...
I think this is a deal-breaker, and arguing about how the pg_notify
directory ought to be writable is not even slightly acceptable --- out
of disk space is an obvious risk. The existing implementation
guarantees that notifications obey ACID: if you commit, they are sent,
and if you don't, they aren't. You can't just put in something that
works most of the time instead.
One possible solution would be to write to the queue before committing
and adding the TransactionID. Then other backends can check if our
TransactionID has successfully committed or not. Not sure if this is
worth the overhead however...
That sounds reasonable, and it's certainly no more overhead than the
tuple visibility checks that happen in the current implementation.
2. The payload parameter is optional. A notifying client can either call
"NOTIFY foo;" or "NOTIFY foo 'payload';". The length of the payload is
currently limited to 128 characters... Not sure if we should allow longer
payload strings...
Might be a good idea to make the max the same as the max length for
prepared transaction GUIDs? Not sure anyone would be shipping those
around, but it's a pre-existing limit of about the same size.
regards, tom lane
I thought of a compromise: add the number of times a notification was
generated (coalesced count+1) to the callback data. That would satisfy
any backwards compatibility concerns and my use case too!
If you are suggesting that the server poke data into the notifier's opaque
payload, I vote no. Maybe the NOTIFY command can include a switch to enable
this behavior. No syntax suggestions at this point.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Andrew Chernow <ac@esilo.com> writes:
I thought of a compromise: add the number of times a notification was
generated (coalesced count+1) to the callback data. That would satisfy
any backwards compatibility concerns and my use case too!
If you are suggesting that the server poke data into the notifier's opaque
payload, I vote no. Maybe the NOTIFY command can include a switch to enable
this behavior. No syntax suggestions at this point.
I agree, we should not have the system modifying the payload string for
this. And don't bother suggesting a third column in the result ---
we'd have to change the FE/BE protocol for that, and it's not going
to happen.
The existing precedent is that the system collapses identical
notifications without payloads. So we could possibly get away with
saying that identical payload-less notifies are collapsed but those
with a payload are not. That doesn't really seem to satisfy the
POLA though. I think Joachim's definition is fine, and anyone who
needs delivery of distinct notifications can easily make his payload
strings unique to ensure it.
regards, tom lane
2. The payload parameter is optional. A notifying client can either call
"NOTIFY foo;" or "NOTIFY foo 'payload';". The length of the payload is
currently limited to 128 characters... Not sure if we should allow longer
payload strings...Might be a good idea to make the max the same as the max length for
prepared transaction GUIDs? Not sure anyone would be shipping those
around, but it's a pre-existing limit of about the same size.
I don't see any reason to impose such a small limit on payload size. Surely
some limit must exist, but 128 characters seems awfully small. I already have
at few places in mind that would require more bytes.
Why not 8K, 64K, 256K, 1M or even more? Is there some other factor in play
forcing this limitation?
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
On Nov 11, 2009, at 10:43 PM, Tom Lane wrote:
Andrew Chernow <ac@esilo.com> writes:
I thought of a compromise: add the number of times a notification
was
generated (coalesced count+1) to the callback data. That would
satisfy
any backwards compatibility concerns and my use case too!If you are suggesting that the server poke data into the
notifier's opaque
payload, I vote no. Maybe the NOTIFY command can include a switch
to enable
this behavior. No syntax suggestions at this point.I agree, we should not have the system modifying the payload string
for
this. And don't bother suggesting a third column in the result ---
we'd have to change the FE/BE protocol for that, and it's not going
to happen.The existing precedent is that the system collapses identical
notifications without payloads. So we could possibly get away with
saying that identical payload-less notifies are collapsed but those
with a payload are not. That doesn't really seem to satisfy the
POLA though. I think Joachim's definition is fine, and anyone who
needs delivery of distinct notifications can easily make his payload
strings unique to ensure it.
The notification count could be a secondary "payload" which does not
affect the first, but I guess I'm the only one complaining about the
coalescing...
-M
On Thu, Nov 12, 2009 at 2:12 AM, Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
Does it cope with the case where a trigger is doing NOTIFY, and you do
a whole-table update, therefore dumping potentially millions of
notifications in at once?(for example a rare maintenance operation on a table which has a
listen/notify arrangement triggered by single inserts or updates)The existing implementation copes with that just fine.
As long as you use the existing way to send out notifications by just
sending "NOTIFY foo", then yes, it will cope with millions of them
just fine because it will collapse them into one notification as does
the current implementation (contrary to the current implementation a
notification will be received from every transaction that has sent one
- the current implementation even collapses notifications from
different transactions).
However if you decide to use the new syntax and add a distinct payload
to every NOTIFY, then you also send out millions of notifications...
With the proposed patch the queue has space for up to about 81,787,680
notifications. The limits are mainly imposed by slru.c which defines
page size, pages per segment and the number of segments available. We
could easily bump that up to a multiple of the current limits if we
have decided that we need to...
Joachim
On Thu, Nov 12, 2009 at 4:25 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
One possible solution would be to write to the queue before committing
and adding the TransactionID. Then other backends can check if our
TransactionID has successfully committed or not. Not sure if this is
worth the overhead however...That sounds reasonable, and it's certainly no more overhead than the
tuple visibility checks that happen in the current implementation.
I am not too concerned about the runtime of the visibility checks,
instead I suppose that most of the overhead will come from waiting for
another transaction to either commit or abort...
If transaction t1 scans the queue and at some point finds
notifications from t2, then it will ask for the status of t2. If it
finds out that t2 is still running, then it has no other option than
to stop working on the queue and wait (it will be signalled again
later once t2 has finished).
This also means that we cannot at the same time write notifications to
the queue and read from it and if a transaction places a few million
notifications into the queue, readers need to wait until it has
finished and only after that they can continue and read the
notifications...
And it means that if the queue is full, we might run into a
deadlock... A transaction adding notifications will wait for the
readers to proceed and the readers wait for the transaction to commit
or abort...
One option could be to write new notifications to a subdirectory, and
create a bunch of new segment files there. Once this is done, the
segment files could be moved over and renamed, so that they continue
the slru queue... If we run out of disk space while filling that
temporary subdirectory, then we can just delete the subdirectory and
nobody has been blocked. We could still run into errors moving and
renaming the segment files (e.g. permission problems) so that we still
might need to abort the transaction...
2. The payload parameter is optional. A notifying client can either call
"NOTIFY foo;" or "NOTIFY foo 'payload';". The length of the payload is
currently limited to 128 characters... Not sure if we should allow longer
payload strings...Might be a good idea to make the max the same as the max length for
prepared transaction GUIDs? Not sure anyone would be shipping those
around, but it's a pre-existing limit of about the same size.
Yes, sounds reasonable to have the same limit for user-defined identifiers...
Joachim
2. The payload parameter is optional. A notifying client can either call
"NOTIFY foo;" or "NOTIFY foo 'payload';". The length of the payload is
currently limited to 128 characters... Not sure if we should allow longer
payload strings...Might be a good idea to make the max the same as the max length for
prepared transaction GUIDs? Not sure anyone would be shipping those
around, but it's a pre-existing limit of about the same size.Yes, sounds reasonable to have the same limit for user-defined identifiers...
[..begging..] Can this be increased significantly? I don't get it, is there any
technical reason to make the limit soo small? This drastically reduces the
usefulness of the payload. I've wanted this feature for quite sometime and it
is quite disappointing that I could not even use it because it is unjustifiably
limited.
One use case I need is making the payload an absolute path, which saves us a
round trip (commonly internet latency) and a query in a section of the system
that's extremely performance sensitive. That sure ain't going to fit in 128
bytes.
I'm sure I'm not the only one who finds this limit too small. I can almost
guarentee complaints would come in if released that way.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
