On 10 August 2016 at 06:24, Michael Paquier <michael.paqu...@gmail.com> wrote:
> On Tue, Aug 9, 2016 at 5:34 PM, Julien Rouhaud
> <julien.rouh...@dalibo.com> wrote:
>> Since 14e8803f1, it's not necessary to acquire the SyncRepLock to see up
>> to date data. But it looks like this commit didn't update all the
>> comment around MyProc->syncRepState, which still mention retrieving the
>> value without and without lock. Also, there's I think a now unneeded
>> test to try to retrieve again syncRepState.
>>
>> Patch attached to fix both small issues, present since 9.5.
>
> You could directly check MyProc->syncRepState and remove syncRepState.
> Could you add it to the next commit fest? I don't think this will get
> into 9.6 as this is an optimization.
Good catch.
I've updated Julien's patch to reflect Michael's suggestion.
Looks good to apply immediately.
14e8803f1 was only a partial patch for the syncrep code, so I don't
see any reason to keep the code as it currently is in 9.5/9.6.
Any objections to backpatching this to 9.5 and 9.6?
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/replication/syncrep.c
b/src/backend/replication/syncrep.c
index 67249d8..47a67cf 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -189,24 +189,16 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
*/
for (;;)
{
- int syncRepState;
-
/* Must reset the latch before testing state. */
ResetLatch(MyLatch);
/*
- * Try checking the state without the lock first. There's no
- * guarantee that we'll read the most up-to-date value, so if
it looks
- * like we're still waiting, recheck while holding the lock.
But if
- * it looks like we're done, we must really be done, because
once
- * walsender changes the state to SYNC_REP_WAIT_COMPLETE, it
will
- * never update it again, so we can't be seeing a stale value
in that
- * case.
+ * Acquiring the lock is not needed, the latch ensures proper
barriers.
+ * If it looks like we're done, we must really be done, because
once
+ * walsender changes the state to SYNC_REP_WAIT_COMPLETE, it
will never
+ * update it again, so we can't be seeing a stale value in that
case.
*/
- syncRepState = MyProc->syncRepState;
- if (syncRepState == SYNC_REP_WAITING)
- syncRepState = MyProc->syncRepState;
- if (syncRepState == SYNC_REP_WAIT_COMPLETE)
+ if (MyProc->syncRepState == SYNC_REP_WAIT_COMPLETE)
break;
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
