|From:||Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>|
|To:||David Rowley <dgrowleyml(at)gmail(dot)com>|
|Cc:||Justin Pryzby <pryzby(at)telsasoft(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Darafei Komяpa Praliaskouski <me(at)komzpa(dot)net>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Michael Banck <mbanck(at)gmx(dot)net>|
|Subject:||Re: Berserk Autovacuum (let's save next Mandrill)|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On Tue, 2020-03-10 at 13:53 +1300, David Rowley wrote:
> 1. Do we need to change documentation on freeze_min_age to mention
> that it does not apply in all cases? I'm leaning towards not changing
> this as `VACUUM FREEZE` is also an exception to this, which I don't
> see mentioned.
I agree with that. Too little documentation is bad, but too much of
it can also confuse and make it hard to find the needle in the haystack.
> 2. Perhaps the documentation in maintenance.sgml should mention that
> the table will be vacuumed with the equivalent of having
> vacuum_freeze_min_age = 0, instead of:
> "Such a vacuum will aggressively freeze tuples."
> aggressive is the wrong word here. We call it an aggressive vacuum if
> we disable page skipping, not for setting the vacuum_freeze_min_age to
Agreed, see below.
> 3. The following DEBUG3 elog should be updated to include the new values:
> elog(DEBUG3, "%s: vac: %.0f (threshold %.0f), anl: %.0f (threshold %.0f)",
> vactuples, vacthresh, anltuples, anlthresh);
> Someone might be confused at why auto-vacuum is running if you don't
> put those in.
> 4. This would be nicer if you swapped the order of the operands to the
> < condition and replaced the operator with >. That'll match the way it
> is done above.
> * If the number of inserted tuples exceeds the threshold and no
> * vacuum is necessary for other reasons, run an "insert-only" vacuum
> * that freezes aggressively.
> if (!(*dovacuum) && vacinsthresh < tabentry->inserts_since_vacuum)
> *dovacuum = true;
> *freeze_all = true;
> It would also be nicer if you assigned the value of
> tabentry->inserts_since_vacuum to a variable, so as to match what the
> other code there is doing. That'll also make the change for #3 neater.
Changed that way.
> 5. The following text:
> A threshold similar to the above is calculated from
> <xref linkend="guc-autovacuum-vacuum-insert-threshold"/> and
> <xref linkend="guc-autovacuum-vacuum-insert-scale-factor"/>.
> Tables that have received more inserts than the calculated threshold
> since they were last vacuumed (and are not eligible for vacuuming for
> other reasons) will be vacuumed to reduce the impact of a future
> anti-wraparound vacuum run.
> I think "... will be vacuumed with the equivalent of having <xref
> linkend="guc-vacuum-freeze-min-age"/> set to <literal>0</literal>".
> I'm not sure we need to mention the reduction of impact to
> anti-wraparound vacuums.
Done like that.
I left in the explanation of the purpose of this setting.
Understanding the purpose of the GUCs will make it easier to tune them
> 6. Please run the regression tests and make sure they pass. The
> "rules" test is currently failing due to the new column in
Oops, sorry. I ran pgindent, but forgot to re-run the regression tests.
Attached is V5, which also fixes the bug discovered my Masahiko Sawada.
He made an interesting suggestion which we should consider before committing.
|Next Message||Laurenz Albe||2020-03-10 19:08:39||Re: Berserk Autovacuum (let's save next Mandrill)|
|Previous Message||Justin Pryzby||2020-03-10 19:01:43||Re: backend type in log_line_prefix?|