On Mon, 2011-02-21 at 18:06 +0900, Fujii Masao wrote:
> Thanks for the patch!
Thanks for the review.
Code available at git://github.com/simon2ndQuadrant/postgres.git
> PREPARE TRANSACTION and ROLLBACK PREPARED should wait for
> replication as well as COMMIT PREPARED?
PREPARE - Yes
ROLLBACK - No
Further discussion welcome
> What if fast shutdown is requested while RecordTransactionCommit
> is waiting in SyncRepWaitForLSN? ISTM fast shutdown cannot complete
> until replication has been successfully done (i.e., until at least one
> synchronous standby has connected to the master especially if
> allow_standalone_primary is disabled). Is this OK?
A "behaviour" - important, though needs further discussion.
> We should emit WARNING when the synchronous standby with
> wal_receiver_status_interval = 0 connects to the master. Because,
> in that case, a transaction unexpectedly would wait for replication
> infinitely.
This can't happen because a WALSender only activates as a sync standby
once it has received a reply from the chosen standby.
> + /* Need a modifiable copy of string */
> + rawstring = pstrdup(SyncRepStandbyNames);
> +
> + /* Parse string into list of identifiers */
> + if (!SplitIdentifierString(rawstring, ',', &elemlist))
>
> pfree(rawstring) and list_free(elemlist) should be called also if
> SplitIdentifierString returns TRUE. Otherwise, memory-leak would
> happen.
Fixed, thanks
> + ereport(FATAL,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("invalid list syntax for parameter
> \"synchronous_standby_names\"")));
> + return false;
>
> "return false" is not required here though that might be harmless.
Compiler likes it.
-- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services