From: Simon Riggs on
On Sun, 2010-02-14 at 17:22 +0100, Joachim Wieland wrote:

> New patch attached, thanks for the review.

Next set of questions

* Will this work during Hot Standby now? The barrier was that it wrote
to a table and so we could not allow that. ISTM this new version can and
should work with Hot Standby. Can you test that and if so, remove the
explicit barrier code and change tests and docs to enable it?

* We also discussed the idea of having a NOTIFY command that would work
from Primary to Standby. All this would need is some code to WAL log the
NOTIFY if not in Hot Standby and for some recovery code to send the
NOTIFY to any listeners on the standby. I would suggest that would be an
option on NOTIFY to WAL log the notification:
e.g. NOTIFY me 'with_payload' FOR STANDBY ALSO;

* Don't really like pg_listening() as a name. Perhaps pg_listening_to()
or pg_listening_on() or pg_listening_for() or pg_listening_channels() or
pg_listen_channels()

* I think it's confusing that pg_notify is both a data structure and a
function. Suggest changing one of those to avoid issues in
understanding. "Use pg_notify" might be confused by a DBA.

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Joachim Wieland on
On Sun, Feb 14, 2010 at 11:44 PM, Simon Riggs <simon(a)2ndquadrant.com> wrote:
> Next set of questions
>
> * Will this work during Hot Standby now? The barrier was that it wrote
> to a table and so we could not allow that. ISTM this new version can and
> should work with Hot Standby. Can you test that and if so, remove the
> explicit barrier code and change tests and docs to enable it?

I have tested it already. The point where it currently fails is the
following line:

qe->xid = GetCurrentTransactionId();

We record the TransactionId (of the notifying transaction) in the
notification in order to later check if this transaction has committed
successfully or not. If you tell me how we can find this out in HS, we
might be done...

The reason why we are doing all this is because we fear that we can
not write the notifications to disk once we have committed to clog...
So we write them to disk before committing to clog and therefore need
to record the TransactionId.


> * We also discussed the idea of having a NOTIFY command that would work
> from Primary to Standby. All this would need is some code to WAL log the
> NOTIFY if not in Hot Standby and for some recovery code to send the
> NOTIFY to any listeners on the standby. I would suggest that would be an
> option on NOTIFY to WAL log the notification:
> e.g. NOTIFY me 'with_payload' FOR STANDBY ALSO;

What should happen if you wanted to replay a NOTIFY WAL record in the
standby but cannot write to the pg_notify/ directory?


> * Don't really like pg_listening() as a name. Perhaps pg_listening_to()
> or pg_listening_on() or pg_listening_for() or pg_listening_channels() or
> pg_listen_channels()

pg_listen_channels() sounds best to me but I leave this decision to a
native speaker.


> * I think it's confusing that pg_notify is both a data structure and a
> function. Suggest changing one of those to avoid issues in
> understanding. "Use pg_notify" might be confused by a DBA.

You are talking about the libpq datastructure PGnotify I suppose... I
don't see it overly confusing but I wouldn't object changing it. There
was a previous discussion about the name, see the last paragraph of
http://archives.postgresql.org/message-id/dc7b844e1002021510i4aaa879fy8bbdd003729d28da(a)mail.gmail.com


Joachim

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Mon, 2010-02-15 at 12:59 +0100, Joachim Wieland wrote:
> On Sun, Feb 14, 2010 at 11:44 PM, Simon Riggs <simon(a)2ndquadrant.com> wrote:
> > Next set of questions
> >
> > * Will this work during Hot Standby now? The barrier was that it wrote
> > to a table and so we could not allow that. ISTM this new version can and
> > should work with Hot Standby. Can you test that and if so, remove the
> > explicit barrier code and change tests and docs to enable it?
>
> I have tested it already. The point where it currently fails is the
> following line:
>
> qe->xid = GetCurrentTransactionId();
>
> We record the TransactionId (of the notifying transaction) in the
> notification in order to later check if this transaction has committed
> successfully or not. If you tell me how we can find this out in HS, we
> might be done...
>
> The reason why we are doing all this is because we fear that we can
> not write the notifications to disk once we have committed to clog...
> So we write them to disk before committing to clog and therefore need
> to record the TransactionId.

That's a shame. So it will never work in Hot Standby mode unless you can
think of a different way.

> > * We also discussed the idea of having a NOTIFY command that would work
> > from Primary to Standby. All this would need is some code to WAL log the
> > NOTIFY if not in Hot Standby and for some recovery code to send the
> > NOTIFY to any listeners on the standby. I would suggest that would be an
> > option on NOTIFY to WAL log the notification:
> > e.g. NOTIFY me 'with_payload' FOR STANDBY ALSO;
>
> What should happen if you wanted to replay a NOTIFY WAL record in the
> standby but cannot write to the pg_notify/ directory?

Same thing that happens to any action that cannot be replayed. Why
should that be a problem?

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Mon, 2010-02-15 at 12:59 +0100, Joachim Wieland wrote:
> > * I think it's confusing that pg_notify is both a data structure and
> a
> > function. Suggest changing one of those to avoid issues in
> > understanding. "Use pg_notify" might be confused by a DBA.
>
> You are talking about the libpq datastructure PGnotify I suppose... I
> don't see it overly confusing but I wouldn't object changing it. There
> was a previous discussion about the name, see the last paragraph of
> http://archives.postgresql.org/message-id/dc7b844e1002021510i4aaa879fy8bbdd003729d28da(a)mail.gmail.com

No, which illustrates the confusion nicely!
Function and datastructure.

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Joachim Wieland on
On Mon, Feb 15, 2010 at 1:48 PM, Simon Riggs <simon(a)2ndquadrant.com> wrote:
> On Mon, 2010-02-15 at 12:59 +0100, Joachim Wieland wrote:
>> I have tested it already. The point where it currently fails is the
>> following line:
>>
>>       qe->xid = GetCurrentTransactionId();
>
> That's a shame. So it will never work in Hot Standby mode unless you can
> think of a different way.

We could probably fake this on the Hot Standby in the following way:

We introduce a commit record for every notifying transaction and write
it into the queue itself. So right before writing anything else, we
write an entry which informs readers that the following records are
not yet committed. Then we write the actual notifications and commit.
In post-commit we return back to the commit record and flip its
status. Reading backends would stop at the commit record and we'd
signal them so that they can continue. This actually plays nicely with
Tom's intent to not have interleaved notifications in the queue (makes
things a bit easier but would probably work either way)...

However we'd need to make sure that we clean up that commit record
even if something weird happens (similar to TransactionIdDidAbort()
returning true) in order to allow the readers to proceed.

Comments?


Joachim

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers