Re: [HACKERS] heap_xlog_lock forgets to reset HEAP_XMAX_INVALID
On 2016-07-15 17:43:44 -0400, Alvaro Herrera wrote: > Andres Freund wrote: > > On 2016-07-15 17:23:57 -0400, Alvaro Herrera wrote: > > > > Maybe we should change fix_infomask_from_infobits() to reset > > > HEAP_XMAX_BITS | HEAP_MOVED too (and HEAP_KEYS_UPDATED I suppose), to > > > avoid doing it in each callsite. > > > > Yea, I was thinking of that as well. But there's code like > > htup->t_infomask &= ~(HEAP_XMAX_BITS | HEAP_MOVED); > > htup->t_infomask2 &= ~HEAP_KEYS_UPDATED; > > if (hot_update) > > HeapTupleHeaderSetHotUpdated(htup); > > else > > HeapTupleHeaderClearHotUpdated(htup); > > fix_infomask_from_infobits(xlrec->old_infobits_set, > > >t_infomask, > > > > >t_infomask2); > > > > so I'd rather only clean this up in master. Well, I think we should move setting hot updated into infomask as well, then rename fix_infomask_from_infobits to set_infomask_from_infobits and such. I want to get this fix and the heap_update stuff in now, before the beta, with time to fix potential fallout. So it's the minimal fix, if I do it... Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] heap_xlog_lock forgets to reset HEAP_XMAX_INVALID
Andres Freund wrote: > On 2016-07-15 17:23:57 -0400, Alvaro Herrera wrote: > > Maybe we should change fix_infomask_from_infobits() to reset > > HEAP_XMAX_BITS | HEAP_MOVED too (and HEAP_KEYS_UPDATED I suppose), to > > avoid doing it in each callsite. > > Yea, I was thinking of that as well. But there's code like > htup->t_infomask &= ~(HEAP_XMAX_BITS | HEAP_MOVED); > htup->t_infomask2 &= ~HEAP_KEYS_UPDATED; > if (hot_update) > HeapTupleHeaderSetHotUpdated(htup); > else > HeapTupleHeaderClearHotUpdated(htup); > fix_infomask_from_infobits(xlrec->old_infobits_set, > >t_infomask, > > >t_infomask2); > > so I'd rather only clean this up in master. Mumble. I don't see any way that this would matter, but I don't object to doing the cleanup in master only. -- Álvaro Herrerahttp://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
Re: [HACKERS] heap_xlog_lock forgets to reset HEAP_XMAX_INVALID
On 2016-07-15 17:23:57 -0400, Alvaro Herrera wrote: > Andres Freund wrote: > > > While validating my patch for > > http://archives.postgresql.org/message-id/20160714060607.klwgq2qr7egt3zrr%40alap3.anarazel.de > > I noticed that the standby's infomask still had HEAP_XMAX_INVALID set > > after replaying a XLOG_HEAP_LOCK record. > > > > That's bad, but not really commonly fatal, because unless prepared > > transactions are used, locks don't need to be present / valid after > > crash-recovery. But it's clearly something we need to fix. > > > > Given that it's been this way for ages, it's not a blocker for > > committing the fix for above URL, but I'll try to get in something today > > for that as well. > > Yes, it looks broken, and the consequences for prepared xacts should be > pretty obvious. Other than those, I think it's pretty innocuous. Yea, I can't see anything else right now. > > Looks like the minimal fix is just to add > > htup->t_infomask &= ~(HEAP_XMAX_BITS | HEAP_MOVED); > > htup->t_infomask2 &= ~HEAP_KEYS_UPDATED; > > to heap_xlog_lock and heap_xlog_lock_updated. > > Maybe we should change fix_infomask_from_infobits() to reset > HEAP_XMAX_BITS | HEAP_MOVED too (and HEAP_KEYS_UPDATED I suppose), to > avoid doing it in each callsite. Yea, I was thinking of that as well. But there's code like htup->t_infomask &= ~(HEAP_XMAX_BITS | HEAP_MOVED); htup->t_infomask2 &= ~HEAP_KEYS_UPDATED; if (hot_update) HeapTupleHeaderSetHotUpdated(htup); else HeapTupleHeaderClearHotUpdated(htup); fix_infomask_from_infobits(xlrec->old_infobits_set, >t_infomask, >t_infomask2); so I'd rather only clean this up in master. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] heap_xlog_lock forgets to reset HEAP_XMAX_INVALID
Andres Freund wrote: > While validating my patch for > http://archives.postgresql.org/message-id/20160714060607.klwgq2qr7egt3zrr%40alap3.anarazel.de > I noticed that the standby's infomask still had HEAP_XMAX_INVALID set > after replaying a XLOG_HEAP_LOCK record. > > That's bad, but not really commonly fatal, because unless prepared > transactions are used, locks don't need to be present / valid after > crash-recovery. But it's clearly something we need to fix. > > Given that it's been this way for ages, it's not a blocker for > committing the fix for above URL, but I'll try to get in something today > for that as well. Yes, it looks broken, and the consequences for prepared xacts should be pretty obvious. Other than those, I think it's pretty innocuous. > Looks like the minimal fix is just to add > htup->t_infomask &= ~(HEAP_XMAX_BITS | HEAP_MOVED); > htup->t_infomask2 &= ~HEAP_KEYS_UPDATED; > to heap_xlog_lock and heap_xlog_lock_updated. Maybe we should change fix_infomask_from_infobits() to reset HEAP_XMAX_BITS | HEAP_MOVED too (and HEAP_KEYS_UPDATED I suppose), to avoid doing it in each callsite. -- Álvaro Herrerahttp://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
