Re: [HACKERS][PATCH] adding simple sock check for windows

On Sun, Apr 1, 2018 at 8:59 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Sat, Mar 31, 2018 at 11:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> ... Oh, just to make things even more fun, PQsocket() returns int, not
>> pgsocket; see its header comment.  Therefore, that test is correctly
>> coded as-is (though it's still useless), and the real problem is that
>> ParallelSlot.sock ought to be declared int not pgsocket.  If you look
>> around at our other places that read PQsocket() and use its result in
>> select() masks, they uniformly use int variables, and I think they're
>> right.
>>
>
> There is one other place where we are using pgsocket, see
> walrcv_receive, but we are using Assert in that place.  I think it
> would be better if the output of PQsocket() can be consistently used
> everywhere.  If there is no particular restriction, then it will be
> good to make it as 'int' everywhere.
>
>
>> Actually, the more I look at this code, the more problems I'm finding.
>> The wait-for-a-free-connection code is just broken on its face, because
>> it supposes it can retry select() waits without reinitializing the
>> FD_SET.  The loop in vacuum_one_database() uses "conn" to call
>> prepare_vacuum_command() without any thought as to whether that is a
>> free connection --- which, typically, it isn't.  It looks to me like
>> libpq silently copes with this silliness, just waiting for the active
>> command to finish and then doing what it's told, but the net result
>> is that supposedly-concurrent vacuums get serialized.
>>
>
> I think it would have been better if this code would have found the
> free_slot before preparing the command and then used the connection
> from free slot.
>

oops, I just saw that you have already pushed a fix for it.  I am not
sure if we should try to do anything about walrcv_receive's output
still uses pgsocket instead of int as the usage in itself doesn't have
any problem.


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com