On Fri, Jan 26, 2024 at 11:44 AM Robert Haas wrote:
> Thanks to you for the patches, and to Peter for participating in the
> discussion which, IMHO, was very helpful in clarifying things.
Glad I could help.
--
Peter Geoghegan
On Fri, Jan 26, 2024 at 11:44 AM Robert Haas wrote:
>
> On Thu, Jan 25, 2024 at 12:25 PM Robert Haas wrote:
> > I think you're probably correct. I just didn't realize what was meant.
>
> I tweaked your v12 based on this discussion and committed the result.
>
> Thanks to you for the patches, and t
On Thu, Jan 25, 2024 at 12:25 PM Robert Haas wrote:
> I think you're probably correct. I just didn't realize what was meant.
I tweaked your v12 based on this discussion and committed the result.
Thanks to you for the patches, and to Peter for participating in the
discussion which, IMHO, was very
On Thu, Jan 25, 2024 at 11:19 AM Melanie Plageman
wrote:
> Cool. I might add "successfully" or "fully" to "Either way, the page
> hasn't been processed yet"
I'm OK with that.
> > > I think it would be nice to clarify this comment. I think the "when
> > > there is little free space anyway" is ref
On Thu, Jan 25, 2024 at 10:19 AM Robert Haas wrote:
>
> On Thu, Jan 25, 2024 at 9:18 AM Melanie Plageman
> wrote:
> > > To me, the first paragraph of this one misses the mark. What I thought
> > > we should be saying here was something like "If we don't have a
> > > cleanup lock, the code above h
On Thu, Jan 25, 2024 at 9:18 AM Melanie Plageman
wrote:
> > To me, the first paragraph of this one misses the mark. What I thought
> > we should be saying here was something like "If we don't have a
> > cleanup lock, the code above has already processed this page to the
> > extent that is possible
On Thu, Jan 25, 2024 at 8:57 AM Robert Haas wrote:
>
> On Wed, Jan 24, 2024 at 9:13 PM Melanie Plageman
> wrote:
> > v12 attached has my attempt at writing better comments for this
> > section of lazy_scan_heap().
>
> + /*
> + * If we didn't get the cleanup lock and the page is not new or empty,
On Wed, Jan 24, 2024 at 9:13 PM Melanie Plageman
wrote:
> v12 attached has my attempt at writing better comments for this
> section of lazy_scan_heap().
+ /*
+ * If we didn't get the cleanup lock and the page is not new or empty,
+ * we can still collect LP_DEAD items in the dead_items array for
On Wed, Jan 24, 2024 at 9:13 PM Melanie Plageman
wrote:
> I didn't incorporate it because I wasn't sure I understood the
> situation. I can imagine us skipping updating the FSM after
> lazy_scan_prune() because there are indexes on the relation and dead
> items on the page and we think we'll do a
On Wed, Jan 24, 2024 at 4:34 PM Melanie Plageman
wrote:
>
> On Wed, Jan 24, 2024 at 2:59 PM Robert Haas wrote:
...
> > If you'd like, I can try rewriting these comments to my satisfaction
> > and you can reverse-review the result. Or you can rewrite them and
> > I'll re-review the result. But I t
On Wed, Jan 24, 2024 at 2:59 PM Robert Haas wrote:
>
> On Thu, Jan 18, 2024 at 9:23 PM Melanie Plageman
> wrote:
> > I have attached a rebased version of the former 0004 as v11-0001.
>
> This looks correct to me, although I wouldn't mind some more eyes on
> it. However, I think that the comments
On Thu, Jan 18, 2024 at 9:23 PM Melanie Plageman
wrote:
> I have attached a rebased version of the former 0004 as v11-0001.
This looks correct to me, although I wouldn't mind some more eyes on
it. However, I think that the comments still need more work.
Specifically:
/*
* Pru
On Thu, Jan 18, 2024 at 3:20 PM Robert Haas wrote:
>
> On Thu, Jan 18, 2024 at 10:09 AM Robert Haas wrote:
> > Oh, I see. Good catch.
> >
> > I've now committed 0001.
>
> I have now also committed 0002 and 0003. I made some modifications to
> 0003. Specifically:
>
> - I moved has_lpdead_items ins
On Thu, Jan 18, 2024 at 10:09 AM Robert Haas wrote:
> Oh, I see. Good catch.
>
> I've now committed 0001.
I have now also committed 0002 and 0003. I made some modifications to
0003. Specifically:
- I moved has_lpdead_items inside the loop over blocks instead of
putting it at the function topleve
On Thu, Jan 18, 2024 at 12:15 PM Peter Geoghegan wrote:
> It's not okay if you fail to update the FSM a second time in the
> second heap pass -- at least in some cases. It's reasonably frequent
> for a page that has 0 usable free space when lazy_scan_prune returns
> to go on to have almost BLCKSZ
On Thu, Jan 18, 2024 at 11:46 AM Robert Haas wrote:
> On Thu, Jan 18, 2024 at 11:17 AM Peter Geoghegan wrote:
> > True. But the way that PageGetHeapFreeSpace() returns 0 for a page
> > with 291 LP_DEAD stubs is a much older behavior. When that happens it
> > is literally true that the page has lo
On Thu, Jan 18, 2024 at 11:17 AM Peter Geoghegan wrote:
> True. But the way that PageGetHeapFreeSpace() returns 0 for a page
> with 291 LP_DEAD stubs is a much older behavior. When that happens it
> is literally true that the page has lots of free space. And yet it's
> not free space we can actual
On Thu, Jan 18, 2024 at 10:43 AM Robert Haas wrote:
> I think we're agreeing but I want to be sure. If we only set LP_DEAD
> items to LP_UNUSED, that frees no space. But if doing so allows us to
> truncate the line pointer array, that that frees a little bit of
> space. Right?
That's part of it,
On Thu, Jan 18, 2024 at 10:42 AM Robert Haas wrote:
> Now, that said, I suspect that we actually could reduce FSM_CATEGORIES
> somewhat without causing any real problems, because many tables are
> going to have tuples that are all about the same size, and even in a
> table where the sizes vary mor
On Thu, Jan 18, 2024 at 10:09 AM Peter Geoghegan wrote:
> The problem with your justification for moving things in that
> direction (if any) is that it is occasionally not quite true: there
> are at least some cases where line pointer truncation after making a
> page's LP_DEAD items -> LP_UNUSED w
On Thu, Jan 18, 2024 at 9:53 AM Melanie Plageman
wrote:
> I believe I've done this in attached v10.
Oh, I see. Good catch.
I've now committed 0001.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, Jan 18, 2024 at 8:52 AM Robert Haas wrote:
> But I also said one more thing that I'd still like to hear your
> thoughts about, which is: why is it right to update the FSM after the
> second heap pass rather than the first one? I can't help but suspect
> this is an algorithmic holdover from
On Wed, Jan 17, 2024 at 5:33 PM Melanie Plageman
wrote:
>
> This does mean that something is not quite right with 0001 as well as
> 0004. We'd end up checking if we are at 8GB much more often. I should
> probably find a way to replicate the cadence on master.
I believe I've done this in attached
On Wed, Jan 17, 2024 at 4:31 PM Peter Geoghegan wrote:
> Actually, I suppose that we couldn't apply it independently of
> nindexes==0. Then we'd call FreeSpaceMapVacuumRange() before our
> second pass over the heap takes place for those LP_DEAD-containing
> heap pages scanned since the last round
On Wed, Jan 17, 2024 at 5:47 PM Melanie Plageman
wrote:
>
> On Wed, Jan 17, 2024 at 4:31 PM Peter Geoghegan wrote:
> >
> > On Wed, Jan 17, 2024 at 4:25 PM Peter Geoghegan wrote:
> > > I tend to suspect that VACUUM_FSM_EVERY_PAGES is fundamentally the
> > > wrong idea. If it's such a good idea th
On Wed, Jan 17, 2024 at 4:31 PM Peter Geoghegan wrote:
>
> On Wed, Jan 17, 2024 at 4:25 PM Peter Geoghegan wrote:
> > I tend to suspect that VACUUM_FSM_EVERY_PAGES is fundamentally the
> > wrong idea. If it's such a good idea then why not apply it all the
> > time? That is, why not apply it indep
On Wed, Jan 17, 2024 at 4:25 PM Peter Geoghegan wrote:
>
> On Wed, Jan 17, 2024 at 3:58 PM Robert Haas wrote:
> > > Ah, I realize I was not clear. I am now talking about inconsistencies
> > > in vacuuming the FSM itself. FreeSpaceMapVacuumRange(). Not updating
> > > the freespace map during the c
On Wed, Jan 17, 2024 at 3:58 PM Robert Haas wrote:
>
> On Wed, Jan 17, 2024 at 3:12 PM Melanie Plageman
> wrote:
>
> > Yes, I also spent some time thinking about this. In master, we do
> > always call lazy_scan_new_or_empty() before calling
> > lazy_scan_noprune(). The code is aiming to ensure we
On Wed, Jan 17, 2024 at 4:25 PM Peter Geoghegan wrote:
> I tend to suspect that VACUUM_FSM_EVERY_PAGES is fundamentally the
> wrong idea. If it's such a good idea then why not apply it all the
> time? That is, why not apply it independently of whether nindexes==0
> in the current VACUUM operation?
On Wed, Jan 17, 2024 at 3:58 PM Robert Haas wrote:
> > Ah, I realize I was not clear. I am now talking about inconsistencies
> > in vacuuming the FSM itself. FreeSpaceMapVacuumRange(). Not updating
> > the freespace map during the course of vacuuming the heap relation.
>
> Fair enough, but I'm sti
On Wed, Jan 17, 2024 at 3:12 PM Melanie Plageman
wrote:
> > Reviewing 0001, consider the case where a table has no indexes.
> > Pre-patch, PageTruncateLinePointerArray() will happen when
> > lazy_vacuum_heap_page() is called; post-patch, it will not occur.
> > That's a loss. Should we care? On the
On Wed, Jan 17, 2024 at 12:17 PM Robert Haas wrote:
>
> On Tue, Jan 16, 2024 at 6:07 PM Melanie Plageman
> wrote:
> > Attached v8 patch set is rebased over this.
>
> Reviewing 0001, consider the case where a table has no indexes.
> Pre-patch, PageTruncateLinePointerArray() will happen when
> lazy
On Tue, Jan 16, 2024 at 6:07 PM Melanie Plageman
wrote:
> Attached v8 patch set is rebased over this.
Reviewing 0001, consider the case where a table has no indexes.
Pre-patch, PageTruncateLinePointerArray() will happen when
lazy_vacuum_heap_page() is called; post-patch, it will not occur.
That's
On Tue, Jan 16, 2024 at 2:23 PM Robert Haas wrote:
>
> On Tue, Jan 16, 2024 at 11:28 AM Melanie Plageman
> wrote:
> > All LGTM.
>
> Committed.
Attached v8 patch set is rebased over this.
In 0004, I've taken the approach you seem to favor and combined the FSM
updates from the prune and no prune
On Tue, Jan 16, 2024 at 4:28 PM Jim Nasby wrote:
> On 1/12/24 12:45 PM, Robert Haas wrote:
> > P.P.S. to everyone: Yikes, this logic is really confusing.
>
> Having studied all this code several years ago when it was even simpler
> - it was *still* very hard to grok even back then. I *greatly
> ap
On 1/12/24 12:45 PM, Robert Haas wrote:
P.P.S. to everyone: Yikes, this logic is really confusing.
Having studied all this code several years ago when it was even simpler
- it was *still* very hard to grok even back then. I *greatly
appreciate* the effort that y'all are putting into increasin
On Tue, Jan 16, 2024 at 11:28 AM Melanie Plageman
wrote:
> All LGTM.
Committed.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Jan 16, 2024 at 10:24 AM Robert Haas wrote:
>
> On Mon, Jan 15, 2024 at 4:03 PM Melanie Plageman
> wrote:
>
> > Perhaps it isn't important, but I find this wording confusing. You
> > mention lazy_scan_prune() and then mention that "the whole test is in
> > one place instead of spread out"
On Mon, Jan 15, 2024 at 4:03 PM Melanie Plageman
wrote:
> It doesn't pass the number of LP_DEAD items back to the caller. It
> passes a boolean.
Oops.
> Perhaps it isn't important, but I find this wording confusing. You
> mention lazy_scan_prune() and then mention that "the whole test is in
> on
On Mon, Jan 15, 2024 at 12:29:57PM -0500, Robert Haas wrote:
> On Fri, Jan 12, 2024 at 4:05 PM Melanie Plageman
> wrote:
> > Yea, that works for now. I mean, I think the way we should do it is
> > update the FSM in lazy_scan_noprune(), but, for the purposes of this
> > patch, yes. has_lpdead_items
On Fri, Jan 12, 2024 at 4:05 PM Melanie Plageman
wrote:
> Yea, that works for now. I mean, I think the way we should do it is
> update the FSM in lazy_scan_noprune(), but, for the purposes of this
> patch, yes. has_lpdead_items output parameter seems fine to me.
Here's v2.
It's not exactly clear
On Fri, Jan 12, 2024 at 3:22 PM Robert Haas wrote:
>
> On Fri, Jan 12, 2024 at 3:04 PM Melanie Plageman
> wrote:
>
> So what's the best way to solve the problem that Peter pointed out?
> Should we pass in the prunestate? Maybe just replace bool
> *recordfreespace with bool *has_lpdead_items?
Yea
On Fri, Jan 12, 2024 at 3:04 PM Melanie Plageman
wrote:
> Also, I think you should combine these in lazy_scan_noprune() now
>
> /* Save any LP_DEAD items found on the page in dead_items array */
> if (vacrel->nindexes == 0)
> {
> /* Using one-pass strategy (since table has no i
On Fri, Jan 12, 2024 at 2:47 PM Robert Haas wrote:
>
> On Fri, Jan 12, 2024 at 2:43 PM Peter Geoghegan wrote:
> > You're using "!prunestate.has_lpdead_items" as part of your test that
> > sets "recordfreespace". But lazy_scan_noprune doesn't get passed a
> > pointer to prunestate, so clearly you'
On Fri, Jan 12, 2024 at 1:52 PM Melanie Plageman
wrote:
> On Fri, Jan 12, 2024 at 1:07 PM Peter Geoghegan wrote:
> > What is "space_freed"? Isn't that something from your uncommitted patch?
>
> Yes, I was mixing the two together.
An understandable mistake.
> I just want to make sure that we agr
On Fri, Jan 12, 2024 at 2:43 PM Peter Geoghegan wrote:
> You're using "!prunestate.has_lpdead_items" as part of your test that
> sets "recordfreespace". But lazy_scan_noprune doesn't get passed a
> pointer to prunestate, so clearly you'll need to detect the same
> condition some other way.
OOPS.
On Fri, Jan 12, 2024 at 2:32 PM Robert Haas wrote:
> On Fri, Jan 12, 2024 at 1:52 PM Melanie Plageman
> wrote:
> This analysis seems correct to me, except that "when
> lazy_scan_noprune() is called" should really say "when
> lazy_scan_noprune() is called (and returns true)", because when it
> ret
On Fri, Jan 12, 2024 at 1:52 PM Melanie Plageman
wrote:
> Yes, I was mixing the two together.
>
> I just want to make sure that we agree that, on master, when
> lazy_scan_prune() is called, the logic for whether or not to update
> the FSM after the first pass is:
>
> indexes == 0 || !has_lpdead_it
On Fri, Jan 12, 2024 at 1:07 PM Peter Geoghegan wrote:
>
> On Fri, Jan 12, 2024 at 12:33 PM Melanie Plageman
> wrote:
> > So, I think this is the logic in master:
> >
> > Prune case, first pass
> >
> > ...
> > - indexes > 0 && (!space_freed || !index_vacuuming) -> update FSM
>
> What is "space_fr
On Fri, Jan 12, 2024 at 1:07 PM Peter Geoghegan wrote:
>
> On Fri, Jan 12, 2024 at 12:33 PM Melanie Plageman
> wrote:
> > So, I think this is the logic in master:
> >
> > Prune case, first pass
> >
> > ...
> > - indexes > 0 && (!space_freed || !index_vacuuming) -> update FSM
>
> What is "space_fr
On Fri, Jan 12, 2024 at 11:50 AM Peter Geoghegan wrote:
> Why do you think that lazy_scan_prune() and lazy_scan_noprune() have
> different ideas about whether to update the FSM or not?
When lazy_scan_prune() is called, we call RecordPageWithFreeSpace if
vacrel->nindexes == 0 && prunestate.has_lpd
On Fri, Jan 12, 2024 at 12:33 PM Melanie Plageman
wrote:
> So, I think this is the logic in master:
>
> Prune case, first pass
>
> ...
> - indexes > 0 && (!space_freed || !index_vacuuming) -> update FSM
What is "space_freed"? Isn't that something from your uncommitted patch?
As I said, the aim i
On Fri, Jan 12, 2024 at 11:50 AM Peter Geoghegan wrote:
>
> On Fri, Jan 12, 2024 at 9:44 AM Robert Haas wrote:
> > Looking through the git history, I see that this behavior seems to
> > date back to 44fa84881fff4529d68e2437a58ad2c906af5805 which introduced
> > lazy_scan_noprune(). The comments do
On Fri, Jan 12, 2024 at 9:44 AM Robert Haas wrote:
>
> On Thu, Jan 11, 2024 at 9:05 PM Melanie Plageman
> wrote:
> > Ah! I think you are right. Good catch. I could fix this with logic like
> > this:
> >
> > bool space_freed = vacrel->tuples_deleted > tuples_already_deleted;
> > if ((vacrel->nind
On Fri, Jan 12, 2024 at 9:44 AM Robert Haas wrote:
> Looking through the git history, I see that this behavior seems to
> date back to 44fa84881fff4529d68e2437a58ad2c906af5805 which introduced
> lazy_scan_noprune(). The comments don't explain the reasoning, but my
> guess is that it was just an ac
On Thu, Jan 11, 2024 at 9:05 PM Melanie Plageman
wrote:
> Yes, this is a good point. Seems like writing maintainable code is
> really about never lying and then figuring out when hiding the truth
> is also lying. Darn!
I think that's pretty true. In this particular case, this code has a
fair numb
On Thu, Jan 11, 2024 at 01:43:27PM -0500, Robert Haas wrote:
> On Tue, Jan 9, 2024 at 2:35 PM Andres Freund wrote:
>> I don't have that strong feelings about it. If both of you think it
>> looks good, go ahead...
>
> Done.
Thanks for e2d5b3b9b643.
--
Michael
signature.asc
Description: PGP sign
On Thu, Jan 11, 2024 at 4:49 PM Robert Haas wrote:
>
> On Thu, Jan 11, 2024 at 2:30 PM Melanie Plageman
> wrote:
>
> I'm still kind of hung up on the changes that 0001 makes to vacuumlazy.c.
>
> Say we didn't add the recordfreespace parameter to lazy_scan_prune().
> Couldn't the caller just compu
On Thu, Jan 11, 2024 at 2:30 PM Melanie Plageman
wrote:
> Attached v7 is rebased over the commit you just made to remove
> LVPagePruneState->hastup.
Apologies for making you rebase but I was tired of thinking about that patch.
I'm still kind of hung up on the changes that 0001 makes to vacuumlaz
Attached v7 is rebased over the commit you just made to remove
LVPagePruneState->hastup.
On Thu, Jan 11, 2024 at 11:54 AM Robert Haas wrote:
>
> On Wed, Jan 10, 2024 at 5:28 PM Melanie Plageman
> wrote:
> > Yes, the options I can think of are:
> >
> > 1) rename the parameter to "immed_reap" or s
On Tue, Jan 9, 2024 at 2:35 PM Andres Freund wrote:
> I don't have that strong feelings about it. If both of you think it looks
> good, go ahead...
Done.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Wed, Jan 10, 2024 at 5:28 PM Melanie Plageman
wrote:
> Yes, the options I can think of are:
>
> 1) rename the parameter to "immed_reap" or similar and make very clear
> in heap_page_prune_opt() that you are not to pass true.
> 2) make immediate reaping work for on-access pruning. I would need a
On Wed, Jan 10, 2024 at 3:54 PM Robert Haas wrote:
>
> On Tue, Jan 9, 2024 at 5:42 PM Melanie Plageman
> wrote:
> > Done in attached v6.
>
> Don't kill me, but:
>
> + /*
> +* For now, pass no_indexes == false
> regardless of whether or not
> +
On Tue, Jan 9, 2024 at 5:42 PM Melanie Plageman
wrote:
> Done in attached v6.
Don't kill me, but:
+ /*
+* For now, pass no_indexes == false
regardless of whether or not
+* the relation has indexes. In the future we
may enable
On Tue, Jan 9, 2024 at 3:40 PM Robert Haas wrote:
>
> On Tue, Jan 9, 2024 at 3:13 PM Melanie Plageman
> wrote:
> > 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]pru
On 1/8/24 2:10 PM, Robert Haas wrote:
On Fri, Jan 5, 2024 at 3:57 PM Andres Freund wrote:
I will be astonished if you can make this work well enough to avoid
huge regressions in plausible cases. There are plenty of cases where
we do a very thorough job opportunistically removing index tuples.
On Tue, Jan 9, 2024 at 3:13 PM Melanie Plageman
wrote:
> 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
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 su
Hi,
On January 9, 2024 11:33:29 AM PST, Robert Haas wrote:
>On Tue, Jan 9, 2024 at 2:23 PM Melanie Plageman
> wrote:
>> Yes, I agree. I thought about it more, and I prefer updating the FSM
>> and setting nonempty_pages into lazy_scan_[no]prune(). Originally, I
>> had ordered the patch set with t
On Tue, Jan 9, 2024 at 2:23 PM Melanie Plageman
wrote:
> Yes, I agree. I thought about it more, and I prefer updating the FSM
> and setting nonempty_pages into lazy_scan_[no]prune(). Originally, I
> had ordered the patch set with that first (before the patch to do
> immediate reaping), but there i
On Tue, Jan 9, 2024 at 1:31 PM Robert Haas wrote:
>
> On Tue, Jan 9, 2024 at 10:56 AM Melanie Plageman
> wrote:
> > Andres had actually said that he didn't like pushing the update of
> > nonempty_pages into lazy_scan_[no]prune(). So, my v4 patch set
> > eliminates this.
>
> Mmph - but I was so lo
On Tue, Jan 9, 2024 at 11:35 AM Melanie Plageman
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, thoug
On Tue, Jan 9, 2024 at 10:56 AM Melanie Plageman
wrote:
> Andres had actually said that he didn't like pushing the update of
> nonempty_pages into lazy_scan_[no]prune(). So, my v4 patch set
> eliminates this.
Mmph - but I was so looking forward to killing hastup!
> On the other hand, the comment
On Mon, Jan 8, 2024 at 3:51 PM Robert Haas wrote:
>
> On Fri, Jan 5, 2024 at 3:34 PM Melanie Plageman
> wrote:
>
> This part of 0002 makes me very, very uncomfortable:
>
> + /*
> +* Update all line pointers per the record, and repair
> fragmentation.
> +
On Tue, Jan 9, 2024 at 12:56 AM Michael Paquier wrote:
>
> On Mon, Jan 08, 2024 at 03:50:47PM -0500, Robert Haas wrote:
> > Hmm, interesting. I haven't had time to study this fully today, but I
> > think 0001 looks fine and could just be committed. Hooray for killing
> > useless variables with dum
On Mon, Jan 08, 2024 at 03:50:47PM -0500, Robert Haas wrote:
> Hmm, interesting. I haven't had time to study this fully today, but I
> think 0001 looks fine and could just be committed. Hooray for killing
> useless variables with dumb names.
I've been looking at 0001 a couple of weeks ago and thou
On Fri, Jan 5, 2024 at 3:34 PM Melanie Plageman
wrote:
> Yes, attached is a patch set which does this. My previous patchset
> already reduced the number of places we unlock the buffer and update
> the freespace map in lazy_scan_heap(). This patchset combines the
> lazy_scan_prune() and lazy_scan_n
On Fri, Jan 5, 2024 at 3:57 PM Andres Freund wrote:
> > I will be astonished if you can make this work well enough to avoid
> > huge regressions in plausible cases. There are plenty of cases where
> > we do a very thorough job opportunistically removing index tuples.
>
> These days the AM is often
On Fri, Jan 5, 2024 at 12:59 PM Melanie Plageman
wrote:
> > But you can just as easily turn this argument on its head, can't you?
> > In general, except for HOT tuples, line pointers are marked dead by
> > pruning and unused by vacuum. Here you want to turn it on its head and
> > make pruning do w
On Fri, Jan 5, 2024 at 12:57 PM Andres Freund wrote:
> > I will be astonished if you can make this work well enough to avoid
> > huge regressions in plausible cases. There are plenty of cases where
> > we do a very thorough job opportunistically removing index tuples.
>
> These days the AM is ofte
On Fri, Jan 5, 2024 at 12:23 PM Robert Haas wrote:
> > As I think we chatted about before, I eventually would like the option to
> > remove index entries for a tuple during on-access pruning, for OLTP
> > workloads. I.e. before removing the tuple, construct the corresponding index
> > tuple, use i
Patch 0001 in the attached set addresses the following review feedback:
- pronto_reap renamed to no_indexes
- reduce the number of callers of heap_prune_record_unused() by calling
it from heap_prune_record_dead() when appropriate
- add unlikely hint to no_indexes test
I've also dropped the patc
On Fri, Jan 5, 2024 at 2:47 PM Andres Freund wrote:
> On 2024-01-04 17:37:27 -0500, Melanie Plageman wrote:
> > On Thu, Jan 4, 2024 at 3:03 PM Andres Freund wrote:
> > >
> > > On 2023-11-17 18:12:08 -0500, Melanie Plageman wrote:
> > > > @@ -972,20 +970,21 @@ lazy_scan_heap(LVRelState *vacrel)
>
Hi,
On 2024-01-05 15:23:12 -0500, Robert Haas wrote:
> On Fri, Jan 5, 2024 at 3:05 PM Andres Freund wrote:
> > An aside:
> >
> > As I think we chatted about before, I eventually would like the option to
> > remove index entries for a tuple during on-access pruning, for OLTP
> > workloads. I.e. be
On Fri, Jan 5, 2024 at 1:47 PM Robert Haas wrote:
>
> On Fri, Jan 5, 2024 at 12:59 PM Melanie Plageman
> wrote:
> MP> I am planning to add a VM update into the freeze record, at which point
> MP> I will move the VM update code into lazy_scan_prune(). This will then
> MP> allow us to consolidate t
On Fri, Jan 5, 2024 at 3:05 PM Andres Freund wrote:
> OTOH, the pruning logic, including its WAL record, already supports marking
> items unused, all we need to do is to tell it to do so in a few more cases. If
> we didn't already need to have support for this, I'd a much harder time
> arguing for
Hi,
On 2024-01-05 08:59:41 -0500, Robert Haas wrote:
> On Thu, Jan 4, 2024 at 6:03 PM Melanie Plageman
> wrote:
> > When a single page is being processed, page pruning happens in
> > heap_page_prune(). Freezing, dead items recording, and visibility
> > checks happen in lazy_scan_prune(). Visibili
Hi,
On 2024-01-04 17:37:27 -0500, Melanie Plageman wrote:
> On Thu, Jan 4, 2024 at 3:03 PM Andres Freund wrote:
> >
> > On 2023-11-17 18:12:08 -0500, Melanie Plageman wrote:
> > > Assert(ItemIdIsNormal(lp));
> > > htup = (HeapTupleHeader) PageGetItem(dp, lp);
> > > @@
On Fri, Jan 5, 2024 at 12:59 PM Melanie Plageman
wrote:
> I actually think we are going to want to stop referring to these steps
> as pruning and vacuuming. It is confusing because vacuuming refers to
> the whole process of doing garbage collection on the table and also to
> the specific step of s
On Fri, Jan 5, 2024 at 8:59 AM Robert Haas wrote:
>
> On Thu, Jan 4, 2024 at 6:03 PM Melanie Plageman
> wrote:
> > When a single page is being processed, page pruning happens in
> > heap_page_prune(). Freezing, dead items recording, and visibility
> > checks happen in lazy_scan_prune(). Visibilit
On Thu, Jan 4, 2024 at 6:03 PM Melanie Plageman
wrote:
> When a single page is being processed, page pruning happens in
> heap_page_prune(). Freezing, dead items recording, and visibility
> checks happen in lazy_scan_prune(). Visibility map updates and
> freespace map updates happen back in lazy_s
On Thu, Jan 4, 2024 at 12:31 PM Robert Haas wrote:
>
> On Wed, Dec 27, 2023 at 11:27 AM Melanie Plageman
> wrote:
> > Do you have specific concerns about its correctness? I understand it
> > is an area where we have to be sure we are correct. But, to be fair,
> > that is true of all the pruning a
Thanks for the review!
On Thu, Jan 4, 2024 at 3:03 PM Andres Freund wrote:
>
> On 2023-11-17 18:12:08 -0500, Melanie Plageman wrote:
> > Assert(ItemIdIsNormal(lp));
> > htup = (HeapTupleHeader) PageGetItem(dp, lp);
> > @@ -715,7 +733,17 @@ heap_prune_chain(Buffer buffe
Hi,
On 2023-11-17 18:12:08 -0500, Melanie Plageman wrote:
> diff --git a/src/backend/access/heap/heapam.c
> b/src/backend/access/heap/heapam.c
> index 14de8158d49..b578c32eeb6 100644
> --- a/src/backend/access/heap/heapam.c
> +++ b/src/backend/access/heap/heapam.c
> @@ -8803,8 +8803,13 @@ heap_xl
Hi,
On 2024-01-04 12:31:36 -0500, Robert Haas wrote:
> On Wed, Dec 27, 2023 at 11:27 AM Melanie Plageman
> wrote:
> > Do you have specific concerns about its correctness? I understand it
> > is an area where we have to be sure we are correct. But, to be fair,
> > that is true of all the pruning a
On Wed, Dec 27, 2023 at 11:27 AM Melanie Plageman
wrote:
> Do you have specific concerns about its correctness? I understand it
> is an area where we have to be sure we are correct. But, to be fair,
> that is true of all the pruning and vacuuming code.
I'm kind of concerned that 0002 might be a p
On Sat, Dec 23, 2023 at 9:14 PM Michael Paquier wrote:
>
> On Mon, Nov 13, 2023 at 07:06:15PM -0500, Melanie Plageman wrote:
> > As best I can tell, our best case scenario is Thomas' streaming read API
> > goes in, we add vacuum as a user, and we can likely remove the skip
> > range logic.
>
> Thi
On Mon, Nov 13, 2023 at 07:06:15PM -0500, Melanie Plageman wrote:
> As best I can tell, our best case scenario is Thomas' streaming read API
> goes in, we add vacuum as a user, and we can likely remove the skip
> range logic.
This does not prevent the work you've been doing in 0001 and 0002
posted
On Fri, Nov 17, 2023 at 6:12 PM Melanie Plageman
wrote:
>
> On Mon, Nov 13, 2023 at 5:28 PM Melanie Plageman
> wrote:
> > When there are no indexes on the relation, we can set would-be dead
> > items LP_UNUSED and remove them during pruning. This saves us a vacuum
> > WAL record, reducing WAL vol
On Mon, Nov 13, 2023 at 5:28 PM Melanie Plageman
wrote:
> When there are no indexes on the relation, we can set would-be dead
> items LP_UNUSED and remove them during pruning. This saves us a vacuum
> WAL record, reducing WAL volume (and time spent writing and syncing
> WAL).
...
> Note that (on p
1 - 100 of 103 matches
Mail list logo