{"push_id":2860968284,"size":5,"distinct_size":1,"ref":"refs/heads/commitfest/19/1004","head":"33eb1ee40437127919dfd22bb014e3d2ca87bcbc","before":"f5e3262ba120908c3ec645c8f22e22c4777b2226","commits":[{"sha":"8582b4d044b05d3fe4bcdf6e039fde8e753934ae","author":{"name":"Michael Paquier","email":"17b9e1c64588c7fa6419b4d29dc1f4426279ba01@paquier.xyz"},"message":"Improve handling of corrupted two-phase state files at recovery\n\nWhen a corrupted two-phase state file is found by WAL replay, be it for\ncrash recovery or archive recovery, then the file is simply skipped and\na WARNING is logged to the user, causing the transaction to be silently\nlost.  Facing an on-disk WAL file which is corrupted is as likely to\nhappen as what is stored in WAL records, but WAL records are already\nable to fail hard if there is a CRC mismatch.  On-disk two-phase state\nfiles, on the contrary, are simply ignored if corrupted.  Note that when\nrestoring the initial two-phase data state at recovery, files newer than\nthe horizon XID are discarded hence no files present in pg_twophase/\nshould be torned and have been made durable by a previous checkpoint, so\nrecovery should never see any corrupted two-phase state file by design.\n\nThe situation got better since 978b2f6 which has added two-phase state\ninformation directly in WAL instead of using on-disk files, so the risk\nis limited to two-phase transactions which live across at least one\ncheckpoint for long periods.  Backups having legit two-phase state files\non-disk could also lose silently transactions when restored if things\nget corrupted.\n\nThis behavior exists since two-phase commit has been introduced, no\nback-patch is done for now per the lack of complaints about this\nproblem.\n\nAuthor: Michael Paquier\nDiscussion: https://postgr.es/m/20180709050309.GM1467@paquier.xyz","distinct":false,"url":"https://api.github.com/repos/postgresql-cfbot/postgresql/commits/8582b4d044b05d3fe4bcdf6e039fde8e753934ae"},{"sha":"f868a8143a98c4172e3faaf415d9352179d5760b","author":{"name":"Tom Lane","email":"ffeaa947aef224da9ba99b6a18bdd74288df0e3e@sss.pgh.pa.us"},"message":"Fix longstanding recursion hazard in sinval message processing.\n\nLockRelationOid and sibling routines supposed that, if our session already\nholds the lock they were asked to acquire, they could skip calling\nAcceptInvalidationMessages on the grounds that we must have already read\nany remote sinval messages issued against the relation being locked.\nThis is normally true, but there's a critical special case where it's not:\nprocessing inside AcceptInvalidationMessages might attempt to access system\nrelations, resulting in a recursive call to acquire a relation lock.\n\nHence, if the outer call had acquired that same system catalog lock, we'd\nfall through, despite the possibility that there's an as-yet-unread sinval\nmessage for that system catalog.  This could, for example, result in\nfailure to access a system catalog or index that had just been processed\nby VACUUM FULL.  This is the explanation for buildfarm failures we've been\nseeing intermittently for the past three months.  The bug is far older\nthan that, but commits a54e1f158 et al added a new recursion case within\nAcceptInvalidationMessages that is apparently easier to hit than any\nprevious case.\n\nTo fix this, we must not skip calling AcceptInvalidationMessages until\nwe have *finished* a call to it since acquiring a relation lock, not\nmerely acquired the lock.  (There's already adequate logic inside\nAcceptInvalidationMessages to deal with being called recursively.)\nFortunately, we can implement that at trivial cost, by adding a flag\nto LOCALLOCK hashtable entries that tracks whether we know we have\ncompleted such a call.\n\nThere is an API hazard added by this patch for external callers of\nLockAcquire: if anything is testing for LOCKACQUIRE_ALREADY_HELD,\nit might be fooled by the new return code LOCKACQUIRE_ALREADY_CLEAR\ninto thinking the lock wasn't already held.  This should be a fail-soft\ncondition, though, unless something very bizarre is being done in\nresponse to the test.\n\nAlso, I added an additional output argument to LockAcquireExtended,\nassuming that that probably isn't called by any outside code given\nthe very limited usefulness of its additional functionality.\n\nBack-patch to all supported branches.\n\nDiscussion: https://postgr.es/m/12259.1532117714@sss.pgh.pa.us","distinct":false,"url":"https://api.github.com/repos/postgresql-cfbot/postgresql/commits/f868a8143a98c4172e3faaf415d9352179d5760b"},{"sha":"f510412df3510e244fb3f9570ba5d2b4d27e773a","author":{"name":"Tom Lane","email":"ffeaa947aef224da9ba99b6a18bdd74288df0e3e@sss.pgh.pa.us"},"message":"Limit depth of forced recursion for CLOBBER_CACHE_RECURSIVELY.\n\nIt's somewhat surprising that we got away with this before.  (Actually,\nsince nobody tests this routinely AFAIK, it might've been broken for\nawhile.  But it's definitely broken in the wake of commit f868a8143.)\nIt seems sufficient to limit the forced recursion to a small number\nof levels.\n\nBack-patch to all supported branches, like the preceding patch.\n\nDiscussion: https://postgr.es/m/12259.1532117714@sss.pgh.pa.us","distinct":false,"url":"https://api.github.com/repos/postgresql-cfbot/postgresql/commits/f510412df3510e244fb3f9570ba5d2b4d27e773a"},{"sha":"361844fe561f6898d90a10382705ad968929a4b2","author":{"name":"Tom Lane","email":"ffeaa947aef224da9ba99b6a18bdd74288df0e3e@sss.pgh.pa.us"},"message":"Save/restore SPI's global variables in SPI_connect() and SPI_finish().\n\nThis patch removes two sources of interference between nominally\nindependent functions when one SPI-using function calls another,\nperhaps without knowing that it does so.\n\nChapman Flack pointed out that xml.c's query_to_xml_internal() expects\nSPI_tuptable and SPI_processed to stay valid across datatype output\nfunction calls; but it's possible that such a call could involve\nre-entrant use of SPI.  It seems likely that there are similar hazards\nelsewhere, if not in the core code then in third-party SPI users.\nPreviously SPI_finish() reset SPI's API globals to zeroes/nulls, which\nwould typically make for a crash in such a situation.  Restoring them\nto the values they had at SPI_connect() seems like a considerably more\nuseful behavior, and it still meets the design goal of not leaving any\ndangling pointers to tuple tables of the function being exited.\n\nAlso, cause SPI_connect() to reset these variables to zeroes/nulls after\nsaving them.  This prevents interference in the opposite direction: it's\npossible that a SPI-using function that's only ever been tested standalone\ncontains assumptions that these variables start out as zeroes.  That was\nthe case as long as you were the outermost SPI user, but not so much for\nan inner user.  Now it's consistent.\n\nReport and fix suggestion by Chapman Flack, actual patch by me.\nBack-patch to all supported branches.\n\nDiscussion: https://postgr.es/m/9fa25bef-2e4f-1c32-22a4-3ad0723c4a17@anastigmatix.net","distinct":false,"url":"https://api.github.com/repos/postgresql-cfbot/postgresql/commits/361844fe561f6898d90a10382705ad968929a4b2"},{"sha":"33eb1ee40437127919dfd22bb014e3d2ca87bcbc","author":{"name":"Commitfest Bot","email":"b9e638e07cc2e2abf56c9c0cad7954f2e3bc1641@cputube.org"},"message":"[CF 19/1004] SERIALIZABLE with parallel query\n\nThis commit was automatically generated by cfbot at commitfest.cputube.org.\nIt is based on patches submitted to the PostgreSQL mailing lists and\nregistered in the PostgreSQL Commitfest application.\n\nThis branch will be overwritten each time a new patch version is posted to\nthe email thread, and also periodically to check for bitrot caused by changes\non the master branch.\n\nCommitfest entry: https://commitfest.postgresql.org/19/1004\nPatch(es): https://www.postgresql.org/message-id/CAEepm=03wAv0ko9qHRiVtx6h5CWd1-L3hqP9XN2oXA__8HtryA@mail.gmail.com\nAuthor(s): Thomas Munro","distinct":true,"url":"https://api.github.com/repos/postgresql-cfbot/postgresql/commits/33eb1ee40437127919dfd22bb014e3d2ca87bcbc"}]}
{"actor":{"display_login":"postgresql-cfbot"}}