From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
---|---|
To: | Greg Sabino Mullane <htamfids(at)gmail(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Peter Geoghegan <pg(at)bowt(dot)ie>, David Rowley <dgrowley(at)gmail(dot)com> |
Subject: | Re: Trigger more frequent autovacuums of heavy insert tables |
Date: | 2024-10-25 15:14:19 |
Message-ID: | CAAKRu_YmQtqnhBxQTDietJCSG0NHuGxmA33ty7KbP6zUA+=pVw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for the review!
On Thu, Oct 24, 2024 at 3:51 PM Greg Sabino Mullane <htamfids(at)gmail(dot)com> wrote:
>
> I really appreciate all the work to make vacuum better. Anything that helps our problem of autovacuum not scaling well for large tables is a win.
>
> I'm not overly familiar with this part of the code base, but here are some questions/ideas:
>
> + /*
> + * Every block marked all-frozen in the VM must also be marked
> + * all-visible.
> + */
> + if (new_rel_allfrozen > new_rel_allvisible)
> + new_rel_allfrozen = new_rel_allvisible;
> +
>
> Maybe tweak either the comment, or the code, as I read that comment as meaning:
>
> if (new_rel_allfrozen > new_rel_allvisible)
> new_ral_allvisible = new_rel_allfrozen;
I've updated it. An all-frozen block must also be all-visible. But not
all-visible blocks are all-frozen
> + /*
> + * If we are modifying relallvisible manually, it is not clear
> + * what relallfrozen value would make sense. Therefore, set it to
> + * -1, or unknown. It will be updated the next time these fields
> + * are updated.
> + */
> + replaces[ncols] = Anum_pg_class_relallfrozen;
> + values[ncols] = Int32GetDatum(-1);
>
> Do we need some extra checks later on when we are actually using this to prevent negative numbers in the calculations? It's only going to make pcnt_unfrozen something like 1.0001 but still might want to skip that.
Great point! I've added this
> In autovacuum.c, seems we could simplify some of the logic there to this?:
>
> if (relpages > 0 && reltuples > 0) {
>
> relallfrozen = classForm->relallfrozen;
> relallvisible = classForm->relallvisible;
>
> if (relallvisible > relpages)
> relallvisible = relpages;
>
> if (relallfrozen > relallvisible)
> relallfrozen = relallvisible;
>
> pcnt_unfrozen = 1 - ((float4) relallfrozen / relpages);
>
> }
> vacinsthresh = (float4) vac_ins_base_thresh + vac_ins_scale_factor * reltuples * pcnt_unfrozen;
I've done something similar to this in attached v2.
> Again, I'm not clear under what circumstances will relallvisible > relpages?
I think this is mostly if someone manually updated the relation stats,
so we clamp it for safety.
- Melanie
Attachment | Content-Type | Size |
---|---|---|
v3-0002-Trigger-more-frequent-autovacuums-with-relallfroz.patch | text/x-patch | 4.7 KB |
v3-0001-Add-relallfrozen-to-pg_class.patch | text/x-patch | 15.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2024-10-25 15:14:37 | Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails |
Previous Message | Melanie Plageman | 2024-10-25 14:29:09 | Re: Can rs_cindex be < 0 for bitmap heap scans? |