Thread: synchronous_commit = apply

On 1 September 2015 at 20:25, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> Hi
>
> Do you think it's reasonable to want to COMMIT a particular transaction on a
> master node, and then immediately run a read-only query on a hot standby
> node that is guaranteed to see that transaction?
>

well, that is important to make load balancing completely safe (not
returning old data when is important to get the latest).
Having said that, i have never seen a case where the apply lag
postgres has really matters or where the cause of the apply lag (I/O)
doesn't get worst if we try to apply immediatly.

Other solutions use a cache on top to apply in-memory at the cost of
getting inconsistent in a failure.

> A friend of mine who works with a different RDBMS technology that can do
> that asked me how to achieve this with Postgres, and I suggested waiting for
> the standby's pg_last_xlog_replay_location() to be >= the master's
> pg_current_xlog_location() after COMMIT, which might involve some looping
> and sleeping.
>
> As a quick weekend learning exercise/hack I recently went looking into how
> we could support $SUBJECT.  I discovered we already report the apply
> progress back to the master, and the synchronous waiting facility seemed to
> be all ready to support this.  In fact it seemed a little too easy so
> something tells me it must be wrong!  But anyway, please see the attached
> toy POC patch which does that.
>

i haven't seen the patch, but probably is as easy as you see it...
IIRC, Simon proposed a patch for this a few years ago and this was
actually contempleted from the beggining in the design of SR.

I guess there were good reasons the patch didn't get applied, i found
this thread and in this one Simon suggest is not the first time he
submitted that option so it should be other threads too:
http://www.postgresql.org/message-id/AANLkTinxoYmWoWBsJxmnpJHJh_YAN9vFmnmhNJDMev4M@mail.gmail.com

--
Jaime Casanova                      www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

On Wed, Sep 2, 2015 at 10:25 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> Hi
>
> Do you think it's reasonable to want to COMMIT a particular transaction on a
> master node, and then immediately run a read-only query on a hot standby
> node that is guaranteed to see that transaction?
>
> A friend of mine who works with a different RDBMS technology that can do
> that asked me how to achieve this with Postgres, and I suggested waiting for
> the standby's pg_last_xlog_replay_location() to be >= the master's
> pg_current_xlog_location() after COMMIT, which might involve some looping
> and sleeping.
>
> As a quick weekend learning exercise/hack I recently went looking into how
> we could support $SUBJECT.  I discovered we already report the apply
> progress back to the master, and the synchronous waiting facility seemed to
> be all ready to support this.  In fact it seemed a little too easy so
> something tells me it must be wrong!  But anyway, please see the attached
> toy POC patch which does that.
>
> The next problem is that the master can be waiting quite a long time for a
> reply from the remote walreceiver containing the desired apply LSN: in the
> best case it learns of apply progress from replies to subsequent unrelated
> records (which might be very soon on a busy system but still involves
> waiting for the next transaction's WAL flush), and in the worst case it
> needs to wait for wal_receiver_status_interval (10 seconds by default),
> which makes for a long COMMIT delay.  I was thinking that the solution to
> that may be to teach StartupLOG to signal the walreceiver after it updates
> XLogCtl->lastReplayedEndRecPtr, which should cause walrcv_receive to be
> interrupted and return early, and then walreceiver could send a reply if it
> sees that lastReplayedEndRecPtr has moved.  Maybe that would generate an
> unacceptably high frequency of signals

One idea is to change the standby so that it manages the locations
that the backends in "apply" mode are waiting for in the master,
and to make the startup process wake the walreceiver up whenever
the replay location reaches either of those locations. In this idea,
walreceiver sends back the "apply" location to the master only when
needed.

Regards,

-- 
Fujii Masao

On Tue, Sep 1, 2015 at 9:25 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> The next problem is that the master can be waiting quite a long time for a
> reply from the remote walreceiver containing the desired apply LSN: in the
> best case it learns of apply progress from replies to subsequent unrelated
> records (which might be very soon on a busy system but still involves
> waiting for the next transaction's WAL flush), and in the worst case it
> needs to wait for wal_receiver_status_interval (10 seconds by default),
> which makes for a long COMMIT delay.  I was thinking that the solution to
> that may be to teach StartupLOG to signal the walreceiver after it updates
> XLogCtl->lastReplayedEndRecPtr, which should cause walrcv_receive to be
> interrupted and return early, and then walreceiver could send a reply if it
> sees that lastReplayedEndRecPtr has moved.  Maybe that would generate an
> unacceptably high frequency of signals, and maybe there is a better form of
> IPC for this.

Yeah, that could be a problem, as could reply volume. If you've got a
bunch of heap inserts of narrow rows into some table, you don't really
want to send a reply after each one.  That would be a lot of replies,
and nobody can really care about them anyway, at least not for
synchronous_commit purposes.  But what if you only sent a signal when
the just-replayed record was a COMMIT record?  I suppose that could
still be a lot of replies on something like a full-tilt pgbench
workload, but even in that case it would help a lot.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Sep 17, 2015 at 12:50 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 1 September 2015 at 20:25, Thomas Munro <thomas.munro@enterprisedb.com>
> wrote:
>> The next problem is that the master can be waiting quite a long time for a
>> reply from the remote walreceiver containing the desired apply LSN: in the
>> best case it learns of apply progress from replies to subsequent unrelated
>> records (which might be very soon on a busy system but still involves
>> waiting for the next transaction's WAL flush), and in the worst case it
>> needs to wait for wal_receiver_status_interval (10 seconds by default),
>> which makes for a long COMMIT delay.  I was thinking that the solution to
>> that may be to teach StartupLOG to signal the walreceiver after it updates
>> XLogCtl->lastReplayedEndRecPtr, which should cause walrcv_receive to be
>> interrupted and return early, and then walreceiver could send a reply if it
>> sees that lastReplayedEndRecPtr has moved.  Maybe that would generate an
>> unacceptably high frequency of signals, and maybe there is a better form of
>> IPC for this.  Without introducing any new IPC, the walreceiver could
>> instead simply report apply progress to the master whenever it sees that the
>> apply LSN has moved after its regular NAPTIME_PER_CYCLE wait (100ms), but
>> that would still introduces bogus latency.  A quick and dirty way to see
>> that on top of the attached patch is to set requestReply = true in
>> WalReceiverMain to force a send after every nap.
>
>
> This problem is exactly why I wrote my recent patch to make WALWriter work
> in recovery.
>
> Currently, the WALReceiver issues regular fsyncs that prevent it from
> replying in time. Also, the WALReceiver waits on incoming data only, so we
> can't (yet) set a latch when the Startup process has applied some records.
>
> I've solved the first problem and know how to solve the second, just haven't
> coded it yet. I was expecting to do that for CF3 or CF4.
>
> I don't think we should be using signals, nor would I expect them to work
> effectively while in an fsync.

That sounds much better.  I had noticed that with my patch the
walreceiver loop was basically trying to do far too much.  I was
contemplating investigating a pipe for IPC, so that it could
select/poll on both the socket connected to master + the new apply
feedback pipe, rather that using raw signals (directly or via latches)
and interrupting syscalls.

>> I can see that using synchronous_commit = apply in the practice might
>> prove difficult:  how does a client know which node is the synchronous
>> standby?  Perhaps those sorts of practical problems are the reason no one
>> has done or wanted this.
>
> It means we need quorum sync rep as well, to make this useful in practice
> without sacrificing HA.
>
> Bringing my patch and Beena's patch together will solve this for us in 9.6

I've been looking at that patch.  It makes sense for adding redundancy
in synchronous_commit = on mode (waiting for WAL flush but not apply).
But it strikes me that to make multi-server synchronous_commit = apply
really useful, it is not enough to wait for a quorum of any N servers
in a group to reply, because a client connected to a given standby
doesn't know whether that standby was one of the N and therefore
whether it is guaranteed to see the effects of a committed transaction
that it has heard about.  Do you have a plan that could address that?

I have been working on a proposal that adds support for reliable
"causal" and "ready-your-writes" consistency, while still allowing for
some number of standbys to fail/fall behind without blocking all
transactions forever.  After a COMMIT with synchronous_commit = apply
returns successfully, you can run a query on any standby node, or tell
another process to run a query on any standby node, and it is
guaranteed to either see the committed transaction or receive a new
error "standby not synchronized".  This behaviour is activated by also
setting synchronous_commit = apply on the standby, and works by adding
some two-way timeout logic.  I will have more to say about this soon
(I have some other work to get out of the way first).

I will not be at all surprised to hear that you already have this
covered and are 18 steps ahead of me!

> So yes, 1) we have thought of it and want it, 2) the basic patch is trivial,
> 3) but it isn't the main problem.

Agreed.  I had a go at this because I needed the trivial plumbing in
so I could work on the more difficult problem above, and I didn't know
you had it in the pipeline already.  I'm glad to hear that you do, and
that you have solved the problem of the interleaving of operations in
walreceiver, and I will be following along with interest.

-- 
Thomas Munro
http://www.enterprisedb.com

Hello, I have some random comments.

At Wed, 16 Sep 2015 23:07:03 +1200, Thomas Munro <thomas.munro@enterprisedb.com> wrote in
<CAEepm=2_dDqQxgGc83_a48rYza3T4P4vPTpSC6xkHcMEoGyspw@mail.gmail.com>
> On Tue, Sep 8, 2015 at 1:11 AM, Thomas Munro <thomas.munro@enterprisedb.com>
> wrote:
> 
> > On Thu, Sep 3, 2015 at 12:02 AM, Fujii Masao <masao.fujii@gmail.com>
> >  wrote:
> > Hmm.  So maybe commit records could have a flag saying 'someone is waiting
> > for this to commit to apply', and the startup process's apply loop would
> > only bother to signal the walreceiver if it sees that flag.  I will try
> > that.
> >
> 
> Here is a version that does that, using a bit in xinfo to request apply
> feedback from standbys when running with synchronous_commit = apply.

The paramter apply_lsn of XLogWalRcvSendReply seems not used in
the function. Maybe

- applyPtr = GetXLogReplayRecPtr(NULL);
+ applyPtr = apply_lsn != InvalidXLogRecPtr ?
+                 apply_lsn : GetXLogReplayRecPtr(NULL);

However, walreceiver already sends feedback containing apply lsn
always so I think it is useless if walreceiver is woke up after
the commit record is applied.

> I am not very happy with the way that xact_redo communicates with the main
> apply loop when it sees that bit, through calls to
> XLogAppliedSynchronousCommit (essentially a global variable), but I
> couldn't immediately see a better way to get information out of xact_redo
> into the apply loop without changing the rm_redo interface.  Perhaps xinfo
> is the wrong place for that information.  Thoughts?

I think it is better to avoid xact_redo_commit to be involved in
the standby side mechanism.

walreceiver don't seem to be the place to read XLogRecord.
StartXOG already parses records in recoveryStopsBefore/After. So
we can do the following thing in place of
XLogAppliedSynchronousCommit() if additional parsing of xlog
records in redo loop is acceptable.

XLogImmediatFeedbackAppliedLSN(XLogReaderState *record)
{   if (XLogRecGetRmid(record) != RM_XACT_ID)      return false;   info = XLogRecGetInfo(record) & XLOG_XACT_OPMASK;
if(xact_info != XLOG_XACT_COMMIT &&       xact_info != XLOG_XACT_COMMIT_PREPARED)       return false;   xl_xact_commit
*xlrec= (xl_xact_commit *) XLogRecGetData(record);   xl_xact_parsed_commit parsed;
ParseCommitRecord(XLogRecGetInfo(record),xlrec, &parsed);   if (! (parsed->xinfo.xinfo &
XACT_XINFO_NEED_APPLY_FEEDBACK))      return false;
 
   WalRcvWakeup();
}



In WalRcvMain, there's a bit too many if(got_SIGUSR1)'s in the
main loop. And the current patch seems to simply double the
walreceiver reply when got_SIGUSR1.



I found one trival mistake,

--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -462,6 +462,11 @@ SyncRepReleaseWaiters(void)        walsndctl->lsn[SYNC_REP_WAIT_FLUSH] = MyWalSnd->flush;
numflush= SyncRepWakeQueue(false, SYNC_REP_WAIT_FLUSH);    }
 
+    if (walsndctl->lsn[SYNC_REP_WAIT_APPLY] < MyWalSnd->apply)
+    {
+        walsndctl->lsn[SYNC_REP_WAIT_APPLY] = MyWalSnd->apply;
+        numflush = SyncRepWakeQueue(false, SYNC_REP_WAIT_APPLY);
+    }

This overwrites numflush by the value which is to be numapply. So
the following DEBUG3 message will be wrong.

> elog(DEBUG3, "released %d procs up to write %X/%X, %d procs up to flush %X/%X",
>      numwrite, (uint32) (MyWalSnd->write >> 32), (uint32) MyWalSnd->write,
>    numflush, (uint32) (MyWalSnd->flush >> 32), (uint32) MyWalSnd->flush);

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

On Fri, Sep 18, 2015 at 7:06 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> Hello, I have some random comments.

Thanks for the feedback!  I have fixed several of the things that you
found in the attached new version -- see comments inline below.
However, I now know that Simon has a better patch in development to do
this, so I won't be developing this further.  (Until that work is
available, this patch is temporarily useful as a prerequisite for
something else that I'm working on so I'll still be using it...)

> At Wed, 16 Sep 2015 23:07:03 +1200, Thomas Munro <thomas.munro@enterprisedb.com> wrote in
<CAEepm=2_dDqQxgGc83_a48rYza3T4P4vPTpSC6xkHcMEoGyspw@mail.gmail.com>
>> On Tue, Sep 8, 2015 at 1:11 AM, Thomas Munro <thomas.munro@enterprisedb.com>
>> wrote:
>>
>> > On Thu, Sep 3, 2015 at 12:02 AM, Fujii Masao <masao.fujii@gmail.com>
>> >  wrote:
>> > Hmm.  So maybe commit records could have a flag saying 'someone is waiting
>> > for this to commit to apply', and the startup process's apply loop would
>> > only bother to signal the walreceiver if it sees that flag.  I will try
>> > that.
>> >
>>
>> Here is a version that does that, using a bit in xinfo to request apply
>> feedback from standbys when running with synchronous_commit = apply.
>
> The paramter apply_lsn of XLogWalRcvSendReply seems not used in
> the function. Maybe
>
> - applyPtr = GetXLogReplayRecPtr(NULL);
> + applyPtr = apply_lsn != InvalidXLogRecPtr ?
> +                 apply_lsn : GetXLogReplayRecPtr(NULL);

You're right, that is what I meant to do.  Fixed.

> However, walreceiver already sends feedback containing apply lsn
> always so I think it is useless if walreceiver is woke up after
> the commit record is applied.

No, XLogWalRcvSendReply only sends feedback sometimes (see the
conditional early returns).

>> I am not very happy with the way that xact_redo communicates with the main
>> apply loop when it sees that bit, through calls to
>> XLogAppliedSynchronousCommit (essentially a global variable), but I
>> couldn't immediately see a better way to get information out of xact_redo
>> into the apply loop without changing the rm_redo interface.  Perhaps xinfo
>> is the wrong place for that information.  Thoughts?
>
> I think it is better to avoid xact_redo_commit to be involved in
> the standby side mechanism.

I agree that this doesn't seem quite right...

> walreceiver don't seem to be the place to read XLogRecord.
> StartXOG already parses records in recoveryStopsBefore/After. So
> we can do the following thing in place of
> XLogAppliedSynchronousCommit() if additional parsing of xlog
> records in redo loop is acceptable.
>
> XLogImmediatFeedbackAppliedLSN(XLogReaderState *record)
> {
>     if (XLogRecGetRmid(record) != RM_XACT_ID)
>        return false;
>     info = XLogRecGetInfo(record) & XLOG_XACT_OPMASK;
>     if (xact_info != XLOG_XACT_COMMIT &&
>         xact_info != XLOG_XACT_COMMIT_PREPARED)
>         return false;
>     xl_xact_commit *xlrec = (xl_xact_commit *) XLogRecGetData(record);
>     xl_xact_parsed_commit parsed;
>     ParseCommitRecord(XLogRecGetInfo(record), xlrec, &parsed);
>     if (! (parsed->xinfo.xinfo & XACT_XINFO_NEED_APPLY_FEEDBACK))
>         return false;
>
>     WalRcvWakeup();
> }

... but I don't think it's a good idea to parse every commit record
twice.  Maybe there could be an XactGetXinfo function which just takes
reads the xinfo field from the front.

> In WalRcvMain, there's a bit too many if(got_SIGUSR1)'s in the
> main loop.

I agree that this control flow is not ideal, but I won't try to
improve that now that I know that Simon has a patch that doesn't use
signals for this and probably rearranges this loop considerably.

> And the current patch seems to simply double the
> walreceiver reply when got_SIGUSR1.

I don't think so -- the pre-existing call to XLogWalRcvSendReply
doesn't send anything unless certain conditions are met.  You can see
this by testing the first version of the patch I posted in this
thread, which didn't do any of this SIGUSR1 stuff -- in that version,
the test program "test-sync-apply --level apply --loops 5" had to wait
~10 seconds for every commit.

> I found one trival mistake,
>
> --- a/src/backend/replication/syncrep.c
> +++ b/src/backend/replication/syncrep.c
> @@ -462,6 +462,11 @@ SyncRepReleaseWaiters(void)
>          walsndctl->lsn[SYNC_REP_WAIT_FLUSH] = MyWalSnd->flush;
>          numflush = SyncRepWakeQueue(false, SYNC_REP_WAIT_FLUSH);
>      }
> +    if (walsndctl->lsn[SYNC_REP_WAIT_APPLY] < MyWalSnd->apply)
> +    {
> +        walsndctl->lsn[SYNC_REP_WAIT_APPLY] = MyWalSnd->apply;
> +        numflush = SyncRepWakeQueue(false, SYNC_REP_WAIT_APPLY);
> +    }
>
> This overwrites numflush by the value which is to be numapply. So
> the following DEBUG3 message will be wrong.

Oops, right.  Fixed.

--
Thomas Munro
http://www.enterprisedb.com