Re: Emit fewer vacuum records by reaping removable tuples during pruning
От | Melanie Plageman |
---|---|
Тема | Re: Emit fewer vacuum records by reaping removable tuples during pruning |
Дата | |
Msg-id | CAAKRu_bcU_2WUhf2hNwKumUzCnx4E7gas9Zb38uJRARZsvk0aw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Emit fewer vacuum records by reaping removable tuples during pruning (Robert Haas <robertmhaas@gmail.com>) |
Ответы |
Re: Emit fewer vacuum records by reaping removable tuples during pruning
(Robert Haas <robertmhaas@gmail.com>)
|
Список | pgsql-hackers |
I had already written the patch for immediate reaping addressing the below feedback before I saw the emails that said everyone is happy with using hastup in lazy_scan_[no]prune() in a preliminary patch. Let me know if you have a strong preference for reordering. Otherwise, I will write the three subsequent patches on top of this one. On Tue, Jan 9, 2024 at 2:00 PM Robert Haas <robertmhaas@gmail.com> wrote: > > On Tue, Jan 9, 2024 at 11:35 AM Melanie Plageman > <melanieplageman@gmail.com> wrote: > > The easiest solution would be to change the name of the parameter to > > heap_page_prune_execute()'s from "no_indexes" to something like > > "validate_unused", since it is only used in assert builds for > > validation. > > Right. > > > However, though I wish a name change was the right way to solve this > > problem, my gut feeling is that it is not. It seems like we should > > rely only on the WAL record itself in recovery. Right now the > > parameter is used exclusively for validation, so it isn't so bad. But > > what if someone uses this parameter in the future in heap_xlog_prune() > > to decide how to modify the page? > > Exactly. > > > It seems like the right solution would be to add a flag into the prune > > record indicating what to pass to heap_page_prune_execute(). In the > > future, I'd like to add flags for updating the VM to each of the prune > > and vacuum records (eliminating the separate VM update record). Thus, > > a new flags member of the prune record could have future use. However, > > this would add a uint8 to the record. I can go and look for some > > padding if you think this is the right direction? > > I thought about this approach and it might be OK but I don't love it, > because it means making the WAL record bigger on production systems > for the sake of assertion that only fires for developers. Sure, it's > possible that there might be another use in the future, but there > might also not be another use in the future. > > How about changing if (no_indexes) to if (ndead == 0) and adding a > comment like this: /* If there are any tuples being marked LP_DEAD, > then the relation must have indexes, so every item being marked unused > must be a heap-only tuple. But if there are no tuples being marked > LP_DEAD, then it's possible that the relation has no indexes, in which > case all we know is that the line pointer shouldn't already be > LP_UNUSED. */ Ah, I like this a lot. Attached patch does this. I've added a modified version of the comment you suggested. My only question is if we are losing something without this sentence (from the old comment): - * ... They don't need to be left in place as LP_DEAD items until VACUUM gets - * around to doing index vacuuming. I don't feel like it adds a lot, but it is absent from the new comment, so thought I would check. > BTW: > > + * LP_REDIRECT, or LP_DEAD items to LP_UNUSED > during pruning. We > + * can't check much here except that, if the > item is LP_NORMAL, it > + * should have storage before it is set LP_UNUSED. > > Is it really helpful to check this here, or just confusing/grasping at > straws? I mean, the requirement that LP_NORMAL items have storage is a > general one, IIUC, not something that's specific to this situation. It > feels like the equivalent of checking that your children don't set > fire to the couch on Tuesdays. Hmm. Yes. I suppose I was trying to find something to validate. Is it worth checking that the line pointer is not already LP_UNUSED? Or is that a bit ridiculous? - Melanie
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Andres FreundДата:
Сообщение: Re: Emit fewer vacuum records by reaping removable tuples during pruning