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
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
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
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
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
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
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:
/*
*
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
>
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.
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
>
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
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
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 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.
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
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?
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
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
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
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
>
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 ||
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
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
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 &&
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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,
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
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
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
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
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
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
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
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
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
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.
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
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
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().
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
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().
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
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
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
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 @@
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
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
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.
>
>
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
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
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
1 - 100 of 103 matches
Mail list logo
