I have some more comments on libpq-failover-8.patch
+ /* FIXME need to check that port is numeric */
Is this still applicable?.
1)
+ /*
+ * if there is more than one host in the connect string and
+ * target_server_type is not specified explicitely, set
+ * target_server_type to "master"
+ */
I think we need comments to know why we change default value based on
number of elements in connection string. why default in not “any" when node
count > 1.
2)
+ /* loop over all the host specs in the node variable */
+ for (node = nodes; node->host != NULL || node->port != NULL; node++)
{
I think instead of loop if we can move these code into a separate function say
pg_add_to_addresslist(host, port, &addrs) this helps in removing temp
variables like "struct node info” and several heap calls around it.
3)
+static char *
+get_port_from_addrinfo(struct addrinfo * ai)
+{
+ char port[6];
+
+ sprintf(port, "%d", htons(((struct sockaddr_in *)
ai->ai_addr)->sin_port));
+ return strdup(port);
+}
Need some comments for this function.
We use strdup in many places no where we handle memory allocation failure.
4)
+ /*
+ * consult connection options and check if RO connection is OK RO
+ * connection is OK if readonly connection is explicitely
+ * requested or if replication option is set, or just one host is
+ * specified in the connect string.
+ */
+ if (conn->pghost == NULL || !conn->target_server_type ||
+ strcmp(conn->target_server_type, "any") == 0)
Comments not in sink with code.
On Wed, Sep 14, 2016 at 12:28 AM, Robert Haas <[email protected]> wrote:
> On Fri, Sep 9, 2016 at 4:49 AM, Victor Wagner <[email protected]> wrote:
> > Random permutation is much more computationally complex than random
> > picking.
>
> It really isn't. The pseudocode given at
> https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle is all of 4
> lines long, and one of those lines is a comment. The C implementation
> is longer, but not by much.
>
> Mind you, I haven't read the patch, so I don't know whether using
> something like this would make the code simpler or more complicated.
> However, if the two modes of operation are (1) try all hosts in random
> order and (2) try all hosts in the listed order, it's worth noting
> that the code for the second thing can be used to implement the first
> thing just by shuffling the list before you begin.
>
> > So, using random permutation instead of random pick wouln't help.
> > And keeping somewhere number of elements in the list wouldn't help
> > either unless we change linked list to completely different data
> > structure which allows random access.
>
> Is there a good reason not to use an array rather than a linked list?
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgsql-hackers mailing list ([email protected])
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
--
Thanks and Regards
Mithun C Y
EnterpriseDB: http://www.enterprisedb.com
