On Thu, Sep 19, 2013 at 12:25 PM, Fujii Masao <masao.fu...@gmail.com> wrote: > On Thu, Sep 19, 2013 at 11:48 AM, Sawada Masahiko <sawada.m...@gmail.com> > wrote: >> I attached the patch which I have modified. > > Thanks for updating the patch! > > Here are the review comments: >
Thank you for reviewing! > I got the compiler warning: > > syncrep.c:112: warning: unused variable 'i' > > How does synchronous_transfer work with synchronous_commit? The currently patch synchronous_transfer doesn't work when synchronous_commit is set 'off' or 'local'. if user changes synchronous_commit value on transaction, checkpointer process can't see it. Due to that, even if synchronous_commit is changed to 'off' from 'on', synchronous_transfer doesn't work. I'm planning to modify the patch so that synchronous_transfer is not affected by 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? > They are unnecessary. I had forgot to remove unnecessary codes. > +#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. > Yes, it seems not necessary to add queue newly. I will delete SYNC_REP_WAIT_DATA_FLUSH and related that. Regards, ------- Sawada Masahiko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers