On Sun, Apr 1, 2018 at 8:59 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Sat, Mar 31, 2018 at 11:33 PM, Tom Lane <t...@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
