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