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_nodes instead 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 nodes are in order as: 1,2,3,4,5,6,7
        
        As per the logic in patch, node 4 with priority 2 will not be added in 
the list whereas 1,2,3 will be added.
        
        The problem is because priority updated for next tracking is not the 
highest priority as of that iteration, it is just priority of last node added 
to the list. So it may happen that a node with       higher priority is 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_names itself. 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



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to