Обсуждение: Re: Understanding when VM record needs snapshot conflict horizon
Hi, On 2025-05-22 18:15:35 -0400, Melanie Plageman wrote: > I'm trying to understand when the visibility map WAL record > (xl_heap_visible) needs to include a snapshot conflict horizon. It needs to be included whenever replaying the WAL record could "break" an existing snapshot on the standby. E.g.: > Currently, when emitting a xl_heap_visible record after phase I of > vacuum, we include a snapshot conflict horizon if the page is being > newly set all-visible in the VM. If a page is marked all visible, snapshots on the standby that still had some of those rows not being visible, would be "corrupted". Thus we need to have a recovery conflict before replaying that record. > We do not include a snapshot conflict horizon in the xl_heap_visible > record if we are newly setting an already all-visible page all-frozen. That seems right to me - if the page is already all visible, freezing xids won't change anything for an existing snapshot, as that snapshot would already consider all the rows to be visible. > I thought this was because if we are setting a page all-visible in the > VM, then we are likely also setting the page level hint PD_ALL_VISIBLE > and thus are likely modifying the page (and perhaps doing so without > emitting WAL), so we should include a conflict horizon in the > subsequent xl_heap_visible record to avoid recovery conflicts. There > is no page-level hint about being all-frozen. > However, there is a comment in the code that says we don't need to > include a conflict horizon when setting an already all-visible page > all-frozen because the snapshot conflict horizon sufficient to make > everything safe for REDO was logged when the page's tuples were > frozen. > > That doesn't make sense to me because: > 1) isn't it possible that a page was entirely frozen but not set all > frozen in the VM for some reason or other and we didn't actually > freeze any tuples in order to set the page all-frozen in the VM and Sure. > 2) if our inclusion of a cutoff_xid when freezing tuples is what makes > it safe to omit it from the VM update, then wouldn't that be true if > we included a cutoff_xid when pruning a page in a way that rendered it > all-visible too? I don't think omitting WAL for VM updates or whatnot is related to the conflict horizon. That's really just for determining whether existing snapshots on the standby conflict with the replay of the record. > For context, I'm writing a patch to add VM update redo to the > xl_heap_prune record, and, in some cases, the record will only contain > an update to the VM and I'm trying to determine when I need a snapshot > conflict horizon in the record. You need to include it if the replay of the record might invalidate existing snapshots. I can't immediately think of a case where that would happen without more than just a VM update. Greetings, Andres Freund
On Fri, May 23, 2025 at 12:04 PM Andres Freund <andres@anarazel.de> wrote: > > > 2) if our inclusion of a cutoff_xid when freezing tuples is what makes > > it safe to omit it from the VM update, then wouldn't that be true if > > we included a cutoff_xid when pruning a page in a way that rendered it > > all-visible too? > > I don't think omitting WAL for VM updates or whatnot is related to the > conflict horizon. That's really just for determining whether existing > snapshots on the standby conflict with the replay of the record. What I was really trying to determine is how much the VM record is responsible for fast-forwarding the snapshot conflict horizon for this page. In my code which seeks to emit a single record for prune/freeze/vm update, I think I need the page visibility cutoff xid to be the snapshot conflict horizon when the record includes a VM update. > > For context, I'm writing a patch to add VM update redo to the > > xl_heap_prune record, and, in some cases, the record will only contain > > an update to the VM and I'm trying to determine when I need a snapshot > > conflict horizon in the record. > > You need to include it if the replay of the record might invalidate existing > snapshots. I can't immediately think of a case where that would happen without > more than just a VM update. Yes, I think this is true. In fact, if you are only updating the VM after pruning/freezing (i.e. making no other modifications to the heap page [including setting PD_ALL_VISIBLE]), I think the rule for a combined prune/freeze/vm record is: 1) if you are not updating the VM, current rules (in master) apply for calculating the snapshot conflict horizon 2) if you updating the VM and you are modifying the heap page at all -- either to prune, freeze, or set PD_ALL_VISIBLE -- then the combined record must have the visibility cutoff xid for the page as its snapshot conflict horizon 3) if you are updating the VM and you are not modifying the heap page at all, then you don't need to include a snapshot conflict horizon in the record because you can safely assume that a record with the visibility cutoff xid for that heap page as the snapshot conflict horizon has already been emitted. And any existing snapshots that would conflict with it would have conflicted with the previous record. I think 3 can only happen if something goes wrong with the VM -- like it is lost somehow. What I am wondering is if it is worth omitting the snapshot conflict horizon in the third case. Currently, you would emit an xl_heap_visible record with InvalidTransactionId as the conflict horizon in this case. But you aren't saving any space and it doesn't seem like you are saving any queries from being canceled by not doing this. It simply makes the logic for what to put in the WAL record more complicated. - Melanie
On Sat, May 24, 2025 at 2:21 AM Melanie Plageman <melanieplageman@gmail.com> wrote: > > On Fri, May 23, 2025 at 12:04 PM Andres Freund <andres@anarazel.de> wrote: > 3) if you are updating the VM and you are not modifying the heap page > at all, then you don't need to include a snapshot conflict horizon in > the record because you can safely assume that a record with the > visibility cutoff xid for that heap page as the snapshot conflict > horizon has already been emitted. And any existing snapshots that > would conflict with it would have conflicted with the previous record. > > I think 3 can only happen if something goes wrong with the VM -- like > it is lost somehow. > > What I am wondering is if it is worth omitting the snapshot conflict > horizon in the third case. > Currently, you would emit an xl_heap_visible record with > InvalidTransactionId as the conflict horizon in this case. But you > aren't saving any space and it doesn't seem like you are saving any > queries from being canceled by not doing this. It simply makes the > logic for what to put in the WAL record more complicated. > IMHO, if we include snapshot conflict horizon in cases where it is not necessary, don't you think it will impact performance on standby? because now it has to loop through the procarray on standby to check whether there is any conflict before applying this WAL. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
On Sun, May 25, 2025 at 6:45 AM Dilip Kumar <dilipbalaut@gmail.com> wrote: > > IMHO, if we include snapshot conflict horizon in cases where it is not > necessary, don't you think it will impact performance on standby? > because now it has to loop through the procarray on standby to check > whether there is any conflict before applying this WAL. Yep, that's a good point. In my patch set to combine the prune/freeze record and visible record, the only time we could omit the snapshot conflict horizon after phase I of vacuum in this combined record is when the heap page was unmodified by phase I and the heap page was already marked all-visible in the VM and is only being set all-frozen. I will make sure that the snapshot conflict horizon is omitted in that case to ensure we don't spend more time on the standby to check for conflicts. - Melanie