I noticed that strerror(errno) wasn't the most helpful error context ever, so I changed it to PQerrorMessage(). There may be room for additional improvement there.
I also noticed that if one connection dies, we terminate the whole thread, and if the thread is serving multiple connections, the other ones are not PQfinished. It may or may not be worthwhile improving this; if pgbench is used to test the server when connections are randomly dropped that would be helpful, otherwise not so much. I ended up backpatching all the way back. Fabien COELHO wrote: > > Hello Alvaro, > > >Any objections to changing it like this? I'd probably backpatch to 9.5, > >but no further (even though this pattern actually appears all the way > >back.) > > My 0.02€: if the pattern is repeated, maybe a function which incorporates > the check would save lines and improve overall readability? > > ... = safePQsocket(...); Hmm, yeah, but that doesn't work too well because we're not invoking exit() in case of an error (which is what the safe pg_malloc etc do), so we'd have to check for what to do after safePQsocket returns -- no improvement in code clarity IMHO. Thanks for the suggestion. Michael Paquier wrote: > Different issues in the same area: > 1) In vacuumdb.c, init_slot() does not check for the return value of > PQsocket(): > slot->sock = PQsocket(conn); > 2) In isolationtester.c, try_complete_step() should do the same. > 3) In pg_recvlogical.c for StreamLogicalLog() I am spotting the same problem. > I guess those ones should be fixed as well, no? Hmm, yeah, perhaps it's worth closing these too. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
