public inbox for [email protected]  
help / color / mirror / Atom feed
From: Masahiko Sawada <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: Melanie Plageman <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Amit Kapila <[email protected]>
Subject: Re: Should vacuum process config file reload more often
Date: Mon, 17 Apr 2023 11:04:22 +0900
Message-ID: <CAD21AoBaBbLTejxYDpHg_oHhXO1RgzKTY1-qj8DjeWGcv9Fnbg@mail.gmail.com> (raw)
In-Reply-To: <CAD21AoD22LdRdvwmiybmOpr+0d1gCEMyOLs9CoU46rycZb-mJg@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CAAKRu_Yg4-kWH_EN7wtpm3z9B+H27UTmOxzzYv1Vg=YdZ6v_rw@mail.gmail.com>
	<CAD21AoCGdxYAJHmE_nXw_MFw1+Qtn3vcThRu5FiadukgQnpWeQ@mail.gmail.com>
	<[email protected]>
	<CAAKRu_aoa0YT36q2n3xa9MZYdepeH4Jyf2EVJj=7YdC_1c974Q@mail.gmail.com>
	<CAAKRu_bSsChKsyEy1ZV2XJQ22FoDdA56efjNr9==m2TO94557w@mail.gmail.com>
	<CAD21AoDi4i4tj1Cf9BG0q2iyVhHisQ2xxVBZajEqExX6t6_b3g@mail.gmail.com>
	<CAAKRu_bkg+0WAGTxsy0HP75RzFbPwqz3ctYq1kThjFhSL69upw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAAKRu_ap7nhmqtKQ1TRo3LzHG3FK=YpGO=O0019M3BbN7ysm3A@mail.gmail.com>
	<[email protected]>
	<CAAKRu_b1HjGCTsFpUnmwLNS8NeXJ+JnrDLhT1osP+Gq9HCU+Rw@mail.gmail.com>
	<[email protected]>
	<CAAKRu_bJ2DJxYoMkyMo9UpoX74LY5d2Jir86QWFUftrGKWP5Bw@mail.gmail.com>
	<CA+TgmoYDYHht9zsHye3tywx7oWGMP4s_6AGefHkDsMuGHLzT7w@mail.gmail.com>
	<CAAKRu_auVL_uE794zwiX6t18LJvKtJ5mrmV9tkbpWzS8AYeChQ@mail.gmail.com>
	<CAAKRu_aX+MLn32kPnAQ30WChbiLTkKKmY9e8+B1ZY1BokpQZYA@mail.gmail.com>
	<CAAKRu_aH-UbvNCQq-EouNUTjtGexQTRVo7ik_vYR4WUocVny7w@mail.gmail.com>
	<CA+TgmoY3EaCz3_7F3+Sb3FUHonf4nxdC-_UWnn6fWEPtXU7t5g@mail.gmail.com>
	<[email protected]>
	<CAAKRu_YmQg_i=0DBfPzy1MMc2GjfuJFo=0rFv4fCzRQStLhjvw@mail.gmail.com>
	<CAAKRu_beQECOV4WfU6nG0hhEVobCnH88Y8MQi1LrTwMfRyxuCw@mail.gmail.com>
	<[email protected]>
	<CAAKRu_Znk6WCCQqEw8hv3+X1Fm8gG0x4KKRAkKmO2JOcA-77-g@mail.gmail.com>
	<[email protected]>
	<CAD21AoBS7o6Ljt_vfqPQPf67AhzKu3fR0iqk8B=vVYczMugKMQ@mail.gmail.com>
	<CAAKRu_bETD+Ari600h6fRjX2p8rJSeMAUp=_y88juqOZgouTSg@mail.gmail.com>
	<[email protected]>
	<CAD21AoD22LdRdvwmiybmOpr+0d1gCEMyOLs9CoU46rycZb-mJg@mail.gmail.com>

On Wed, Apr 12, 2023 at 12:05 AM Masahiko Sawada <[email protected]> wrote:
>
> On Fri, Apr 7, 2023 at 10:23 PM Daniel Gustafsson <[email protected]> wrote:
> >
> > > On 7 Apr 2023, at 15:07, Melanie Plageman <[email protected]> wrote:
> > > On Fri, Apr 7, 2023 at 2:53 AM Masahiko Sawada <[email protected]> wrote:
> > >
> > > Definitely seems worth fixing as it kind of defeats the purpose of the
> > > original commit. I wish I had noticed before!
> > >
> > > Your fix has:
> > >            !(avopts && (avopts->vacuum_cost_limit >= 0 ||
> > >                        avopts->vacuum_cost_delay >= 0));
> > >
> > > And though delay is required to be >= 0
> > >                        avopts->vacuum_cost_delay >= 0
> > >
> > > Limit does not. It can just be > 0.
> > >
> > > postgres=# create table foo (a int) with (autovacuum_vacuum_cost_limit = 0);
> > > ERROR:  value 0 out of bounds for option "autovacuum_vacuum_cost_limit"
> > > DETAIL:  Valid values are between "1" and "10000".
> > >
> > > Though >= is also fine, the rest of the code in all versions always
> > > checks if limit > 0 and delay >= 0 since 0 is a valid value for delay
> > > and not for limit. Probably best we keep it consistent (though the whole
> > > thing is quite confusing).
> >
> > +1
>
> +1. I misunderstood the initial value of autovacuum_vacuum_cost_limit reloption.

I've attached an updated patch for fixing at_dobalance condition.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com


Attachments:

  [application/octet-stream] 0001-Fix-the-condition-of-joining-autovacuum-workers-to-b.patch (1.2K, ../CAD21AoBaBbLTejxYDpHg_oHhXO1RgzKTY1-qj8DjeWGcv9Fnbg@mail.gmail.com/2-0001-Fix-the-condition-of-joining-autovacuum-workers-to-b.patch)
  download | inline diff:
From 875e0ca1965fb371a982d0ed9b805c45faeeab22 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <[email protected]>
Date: Mon, 17 Apr 2023 10:59:17 +0900
Subject: [PATCH] Fix the condition of joining autovacuum workers to balance
 calculation.

Previously, we exclude an autovacuum worker from balance calculation
if the table's autovacuum_vacuum_cost_delay storage option is >
0. However, since the initial value and minimum value of
autovacuum_vacuum_cost_delay storage option are -1 and 0 respectively,
we should exclude it if the table's autovacuum_vacuum_cost_delay
storage option is >= 0.
---
 src/backend/postmaster/autovacuum.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 53c8f8d79c..33d80b067b 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2952,7 +2952,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 		 */
 		tab->at_dobalance =
 			!(avopts && (avopts->vacuum_cost_limit > 0 ||
-						 avopts->vacuum_cost_delay > 0));
+						 avopts->vacuum_cost_delay >= 0));
 	}
 
 	heap_freetuple(classTup);
-- 
2.31.1



view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Should vacuum process config file reload more often
  In-Reply-To: <CAD21AoBaBbLTejxYDpHg_oHhXO1RgzKTY1-qj8DjeWGcv9Fnbg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox