On Thu, Mar 21, 2019 at 9:26 PM Simon Riggs <si...@2ndquadrant.com> wrote: > On Thu, 13 Dec 2018 at 14:48, Alexander Korotkov <aekorot...@gmail.com> wrote: >> On Thu, Dec 13, 2018 at 10:46 PM Andres Freund <and...@anarazel.de> wrote: >> > On 2018-12-13 22:40:59 +0300, Alexander Korotkov wrote: >> > > It doesn't mater, because we release all locks on every buffer at one >> > > time. The unlock order can have effect on what waiter will acquire >> > > the lock next after ginRedoDeletePage(). However, I don't see why one >> > > unlock order is better than another. Thus, I just used the rule of >> > > thumb to not change code when it's not necessary for bug fix. >> > >> > I think it's right to not change unlock order at the same time as a >> > bugfix here. More generally I think it can often be useful to default >> > to release locks in the inverse order they've been acquired - if there's >> > any likelihood that somebody will acquire them in the same order, that >> > ensures that such a party would only need to wait for a lock once, >> > instead of being woken up for one lock, and then immediately having to >> > wait for the next one. >> >> Good point, thank you! > > > It's been pointed out to me that 52ac6cd2d0cd70e01291e0ac4ee6d068b69bc478 > introduced a WAL incompatibility that has not been flagged. > > In ginRedoDeletePage() we use the struct directly to read the WAL record, so > if a WAL record was written prior to > 52ac6cd2d0cd70e01291e0ac4ee6d068b69bc478, yet read by code at > 52ac6cd2d0cd70e01291e0ac4ee6d068b69bc478 or later then we will have problems, > since deleteXid will not be set correctly. > > It seems this should not have been backpatched. > > Please give your assessment.
Oh, right. This is my fault. However, I think this still can be backpatched correctly. We can determine whether xlog record data contains deleteXid by its size. See the attached patch. I haven't test this yet. I'm going to test it. If OK, then push. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
gin-redo-delete-page-backpatch-fix.patch
Description: Binary data
