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 <robertmh...@gmail.com> wrote: > On Fri, Sep 9, 2016 at 4:49 AM, Victor Wagner <vi...@wagner.pp.ru> 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 (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Thanks and Regards Mithun C Y EnterpriseDB: http://www.enterprisedb.com