Обсуждение: pgsql: Fix page modification outside of critical section in GIN
Fix page modification outside of critical section in GIN By oversight 52ac6cd2d0 makes ginDeletePage() sets pd_prune_xid of page to be deleted before entering the critical section. It appears that only versions 11 and later were affected by this oversight. Backpatch-through: 11 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/b107140804817cc30a4069b1bb5545aa3ea0ce6c Modified Files -------------- src/backend/access/gin/ginvacuum.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Alexander Korotkov <akorotkov@postgresql.org> writes: > Fix page modification outside of critical section in GIN > By oversight 52ac6cd2d0 makes ginDeletePage() sets pd_prune_xid of page to be > deleted before entering the critical section. It appears that only versions 11 > and later were affected by this oversight. I happened to notice, while reviewing stuff for the release notes, that this patch does not do what the commit message says. The previous code modified the pd_prune_xid of the "dBuffer" page, but now it's modifying the pd_prune_xid of the "lBuffer" page. Is that actually correct? If it isn't, maybe you need to move the GinPageSetDeleteXid call down a bit further, next to the GinPageSetDeleted call (by which time the "page" variable has been restored to the right thing). Another idea which would be safer if a bit more invasive is to stop using the single "page" variable for two purposes. regards, tom lane
I wrote: > Alexander Korotkov <akorotkov@postgresql.org> writes: >> Fix page modification outside of critical section in GIN > I happened to notice, while reviewing stuff for the release notes, > that this patch does not do what the commit message says. The > previous code modified the pd_prune_xid of the "dBuffer" page, > but now it's modifying the pd_prune_xid of the "lBuffer" page. > Is that actually correct? Since we're rapidly approaching the wrap deadline for this week's releases, I took it upon myself to review this code more carefully, and concluded that indeed it isn't correct. I pushed a fix that moves the GinPageSetDeleteXid call again. regards, tom lane
On Sun, Feb 9, 2020 at 8:05 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > I wrote: > > Alexander Korotkov <akorotkov@postgresql.org> writes: > >> Fix page modification outside of critical section in GIN > > > I happened to notice, while reviewing stuff for the release notes, > > that this patch does not do what the commit message says. The > > previous code modified the pd_prune_xid of the "dBuffer" page, > > but now it's modifying the pd_prune_xid of the "lBuffer" page. > > Is that actually correct? > > Since we're rapidly approaching the wrap deadline for this week's > releases, I took it upon myself to review this code more carefully, > and concluded that indeed it isn't correct. I pushed a fix that > moves the GinPageSetDeleteXid call again. Sorry for delayed response. Yes, my fix for oversight contain oversight itself. I confirm the fix you've committed is correct. Thank you! ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Alexander Korotkov <a.korotkov@postgrespro.ru> writes: > Sorry for delayed response. Yes, my fix for oversight contain > oversight itself. I confirm the fix you've committed is correct. > Thank you! Thanks for checking! regards, tom lane