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.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com