Re: Support for N synchronous standby servers

On 09 August 2014 11:33, Michael Paquier Wrote:

> Please find attached a patch to add support of synchronous replication
> for multiple standby servers. This is controlled by the addition of a
> new GUC parameter called synchronous_standby_num, that makes server
> wait for transaction commit on the first N standbys defined in
> synchronous_standby_names. The implementation is really straight-
> forward, and has just needed a couple of modifications in walsender.c
> for pg_stat_get_wal_senders and syncrep.c.

I have just started looking into this patch.
Please find below my first level of observation from the patch:

1. Allocation of memory for sync_nodes in function SyncRepGetSynchronousNodes should be equivalent to
allowed_sync_nodesinstead of max_wal_senders. As anyway we are not going to store sync stdbys more   than
allowed_sync_nodes.   sync_nodes = (int *) palloc(allowed_sync_nodes * sizeof(int)); 

2. Logic of deciding the highest priority one seems to be in-correct.Assume, s_s_num = 3, s_s_names = 3,4,2,1 standby
nodesare in order as: 1,2,3,4,5,6,7As per the logic in patch, node 4 with priority 2 will not be added in the list
whereas1,2,3 will be added.The problem is because priority updated for next tracking is not the highest priority as of
thatiteration, it is just priority of last node added to the list. So it may happen that a node with     higher
priorityis still there in list but we are comparing with some other smaller priority.  
3. Can we optimize the function SyncRepGetSynchronousNodes in such a way that it gets the number of standby nodes from
s_s_namesitself. With this it will be usful to stop scanning the moment we get first s_s_num potential standbys. 

Thanks and Regards,
Kumar Rajeev Rastogi