{"push_id":2447996938,"size":1,"distinct_size":1,"ref":"refs/heads/REL9_6_STABLE","head":"6cd110c477b483cee7fc4b3ce0fdfb961dbe39b9","before":"91d82317d252e41885c06597c394d25845faeafc","commits":[{"sha":"6cd110c477b483cee7fc4b3ce0fdfb961dbe39b9","author":{"name":"Tom Lane","email":"9ca635a44fdf5aa326758d8d245579b9ae84653c@sss.pgh.pa.us"},"message":"Fix assorted issues in parallel vacuumdb.\n\nAvoid storing the result of PQsocket() in a pgsocket variable; it's\ndeclared as int, and the no-socket test is properly written as \"x < 0\"\nnot \"x == PGINVALID_SOCKET\".  This accidentally had no bad effect\nbecause we never got to init_slot() with a bad connection, but it's\nstill wrong.\n\nActually, it seems like we should avoid storing the result for a long\nperiod at all.  The function's not so expensive that it's worth avoiding,\nand the existing coding technique here would fail if anyone tried to\nPQreset the connection during the life of the program.  Hence, just\nre-call PQsocket every time we construct a select(2) mask.\n\nSpeaking of select(), GetIdleSlot imagined that it could compute the\nselect mask once and continue to use it over multiple calls to\nselect_loop(), which is pretty bogus since that would stomp on the\nmask on return.  This could only matter if the function's outer loop\niterated more than once, which is unlikely (it'd take some connection\nreceiving data, but not enough to complete its command).  But if it\ndid happen, we'd acquire \"tunnel vision\" and stop watching the other\nconnections for query termination, with the effect of losing parallelism.\n\nAnother way in which GetIdleSlot could lose parallelism is that once\nPQisBusy returns false, it would lock in on that connection and do\nPQgetResult until that returns NULL; in some cases that could result\nin blocking.  (Perhaps this can never happen in vacuumdb due to the\nlimited set of commands that it can issue, but I'm not quite sure\nof that, and even if true today it's not a future-proof assumption.)\nRefactor the code to do that properly, so that it risks blocking in\nPQgetResult only in cases where we need to wait anyway.\n\nAnother loss-of-parallelism problem, which *is* easily demonstrable,\nis that any setup queries issued during prepare_vacuum_command() were\nalways issued on the last-to-be-created connection, whether or not\nthat was idle.  Long-running operations on that connection thus\nprevented issuance of additional operations on the other ones, except\nin the limited cases where no preparatory query was needed.  Instead,\nwait till we've identified a free connection and use that one.\n\nAlso, avoid core dump due to undersized malloc request in the case\nthat no tables are identified to be vacuumed.\n\nThe bogus no-socket test was noted by CharSyam, the other problems\nidentified in my own code review.  Back-patch to 9.5 where parallel\nvacuumdb was introduced.\n\nDiscussion: https://postgr.es/m/CAMrLSE6etb33-192DTEUGkV-TsvEcxtBDxGWG1tgNOMnQHwgDA@mail.gmail.com","distinct":true,"url":"https://api.github.com/repos/Aloisius/postgresql/commits/6cd110c477b483cee7fc4b3ce0fdfb961dbe39b9"}]}
{"actor":{"display_login":"Aloisius"}}