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

From Rady, Doug
Subject Re: PATCH: pgbench - option to build using ppoll() for largerconnection counts
Date
Msg-id 81936381-841A-459D-9FE5-18659680355C@amazon.com
Whole thread Raw
In response to Re: PATCH: pgbench - option to build using ppoll() for largerconnection counts  (Fabien COELHO <coelho@cri.ensmp.fr>)
Responses Re: PATCH: pgbench - option to build using ppoll() for largerconnection counts
List pgsql-hackers
On 1/25/18, 14:46, "Fabien COELHO" <coelho@cri.ensmp.fr> wrote:

    
    Hello Doug,

Hello Fabien,
    
    > 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.

Made a distinct function for each section.
  
    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.

Duplicated.
    
    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.

Added comment. Changed output to be more compatible with socket() error check.
Extra ppoll() specific error info only output when debug flag set ... not sure if this is OK or should just remove the
extrainfo.
 
    
    
    > [...] 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.

Added option to force use of select(2) via:  -DUSE_SELECT
    
    -- 
    Fabien.
    
Thank you!!
doug



    


Attachment

pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: [HACKERS] Refactoring identifier checks to consistently use strcmp
Next
From: Daniel Gustafsson
Date:
Subject: Re: [HACKERS] Support for Secure Transport SSL library on macOS as OpenSSL alternative