Re: PATCH: pgbench - option to build using ppoll() for largerconnection counts - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: PATCH: pgbench - option to build using ppoll() for largerconnection counts
Date
Msg-id alpine.DEB.2.20.1801252339520.26762@lancre
Whole thread Raw
In response to Re: PATCH: pgbench - option to build using ppoll() for largerconnection counts  ("Rady, Doug" <radydoug@amazon.com>)
Responses Re: PATCH: pgbench - option to build using ppoll() for largerconnection counts
List pgsql-hackers
Hello Doug,

> This time with the revised patch file:  pgbench11-ppoll-v8.patch

Patch applies cleanly. Compiles cleanly and runs fine in both ppoll & 
select cases.

I'm okay with having a preferred ppoll implementation because of its improved
capability.

A few minor additional comments/suggestions:

Cpp has an #elif that could be used to manage the ppoll/select alternative.
It is already used elsewhere in the file. Or not.

I must admit that I'm not fond of the alloc_socket_set trick with MAXCLIENTS,
especially without any comment. I'd suggest to just have two distinct functions
in their corresponding sections.

I would add a comment that free_socket_set code is common to both 
versions, and maybe consider moving it afterwards. Or maybe just duplicate 
if in each section for homogeneity.

It looks like error_on_socket and ignore_socket should return a boolean instead
of an int. Also, maybe simplify the implementation of the later by avoiding
the ?: expression.

ISTM that the error_on_socket function in the ppoll case deserves some 
comments, especially on the condition.


> [...] Replaced USE_PPOLL with HAVE_PPOLL as having both seems redundant.

I'm okay with that. I'm wondering whether there should be a way to force 
using one or the other when both are available. Not sure.

-- 
Fabien.


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
Next
From: Daniel Gustafsson
Date:
Subject: Re: [HACKERS] Optional message to user when terminating/cancelling backend