Re: Patch for fail-back without fresh backup - Mailing list pgsql-hackers

On Thu, Sep 19, 2013 at 11:48 AM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:
> I attached the patch which I have modified.

Thanks for updating the patch!

Here are the review comments:

I got the compiler warning:
   syncrep.c:112: warning: unused variable 'i'

How does synchronous_transfer work with synchronous_commit?

+ * accept all the likely variants of "off".

This comment should be removed because synchronous_transfer
doesn't accept the value "off".

+    {"commit", SYNCHRONOUS_TRANSFER_COMMIT, true},

ISTM the third value "true" should be "false".

+    {"0", SYNCHRONOUS_TRANSFER_COMMIT, true},

Why is this needed?

+        elog(WARNING, "XLogSend sendTimeLineValidUpto(%X/%X) <=
sentPtr(%X/%X) AND sendTImeLine",
+             (uint32) (sendTimeLineValidUpto >> 32), (uint32)
sendTimeLineValidUpto,
+             (uint32) (sentPtr >> 32), (uint32) sentPtr);

Why is this needed?

+#define SYNC_REP_WAIT_FLUSH                    1
+#define SYNC_REP_WAIT_DATA_FLUSH    2

Why do we need to separate the wait-queue for wait-data-flush
from that for wait-flush? ISTM that wait-data-flush also can
wait for the replication on the wait-queue for wait-flush, and
which would simplify the patch.

Regards,

-- 
Fujii Masao