From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Rajeev rastogi <rajeev(dot)rastogi(at)huawei(dot)com> |
Cc: | PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Support for N synchronous standby servers |
Date: | 2014-08-22 14:42:35 |
Message-ID: | CAB7nPqQ0X62qx-5E3UDyzjd0xg=gR-LK85jNr_gsVGu3Vs6-Yg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Aug 22, 2014 at 7:14 PM, Rajeev rastogi <rajeev(dot)rastogi(at)huawei(dot)com>
wrote:
> I have just started looking into this patch.
> Please find below my first level of observation from the patch:
>
Thanks! Updated patch attached.
> 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));
>
Fixed.
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.
>
Fixed. Nice catch!
> 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.
>
By doing so, we would need to scan the WAL sender array more than once (or
once if we can find N sync nodes with a name matching the first entry, smth
unlikely to happen). We would need as well to recalculate for a given item
in the list _names what is its priority and compare it with the existing
entries in the WAL sender list. So this is not worth the shot.
Also, using the priority instead of s_s_names is more solid as s_s_names is
now used only in SyncRepGetStandbyPriority to calculate the priority for a
given WAL sender, and is a function only called by a WAL sender itself when
it initializes.
Regards,
--
Michael
Attachment | Content-Type | Size |
---|---|---|
20140822_multi_syncrep_v6.patch | text/x-patch | 27.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Kevin Grittner | 2014-08-22 14:47:39 | Re: delta relations in AFTER triggers |
Previous Message | Alvaro Herrera | 2014-08-22 14:41:55 | Re: Proposal to add a QNX 6.5 port to PostgreSQL |