Re: Sync Rep v17

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