Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tveeS-006n1G-PP for pgsql-hackers@arkaria.postgresql.org; Fri, 21 Mar 2025 15:51:16 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1tveeP-00A9Ol-Qc for pgsql-hackers@arkaria.postgresql.org; Fri, 21 Mar 2025 15:51:13 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tveeP-00A9Oc-Cb for pgsql-hackers@lists.postgresql.org; Fri, 21 Mar 2025 15:51:13 +0000 Received: from mail-ot1-x330.google.com ([2607:f8b0:4864:20::330]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1tveeM-000LOQ-2q for pgsql-hackers@postgresql.org; Fri, 21 Mar 2025 15:51:13 +0000 Received: by mail-ot1-x330.google.com with SMTP id 46e09a7af769-72bbd3a3928so1291456a34.2 for ; Fri, 21 Mar 2025 08:51:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1742572269; x=1743177069; darn=postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=RP3sWU16UYNQjytuDKP14Am9x5+2vv/Snz63pncYEPg=; b=SaH7MCv4oE0S/As4xcaof+I2Ap79pA4fCCqSOAkndacX3ksqEoU0Oa9ZYUdYX0t7I/ oAwME99bMXw3E+nrLYskVHkKgC0TApuNSnmZ7DhYLijNXDuN1lU6dWcKmc41ANtk3aoV TSm9Ksg1tDn+9h+/mi1QusRAkerh/tBG+zaldEyQgbF9PUD5G2Jt74/qQc9TtQamG/5U aiKuTK9ccGnCayf1PpwJ/xoynj39FSBCPmJmG8uJ9rUWl7iK4tGuNdzj+TseP4mKKPDJ Cvlt4Ajc/m778I9IGaf/+knLu2LjOJo7IyQIcTZq/ZN8t6JWZK4QXzYz2HEh8GyiIktX aHYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1742572269; x=1743177069; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=RP3sWU16UYNQjytuDKP14Am9x5+2vv/Snz63pncYEPg=; b=H3R0YR9lUB7MQQhnzcPos+d73uZPdqB/uT/y0CZmT6dBHvRNcvhbRlMhexgwVfhE3o yQjTJYJz3RHk5U2cZkNsoCL+ZPsgfEyX7dq7E8I+nxJbKAKvpg4doxqcqPPNs7Cyz0CC s0RUA7UsRoxZ/ILg3yFyPRfCOH3b8ptlKcuLxrVI1inPpv88X9yy33TYeKOfCSTbBIok t7oTUKFZN/NgecZfXv7WP1D2S250iS9QJ2ElrWCsWsyljne/ujmGuhvAVMPD4noifmdr ZGG0brF/d9fFuvvOhmCjzW94hmIHotDxcm0epIXhdFREFPH4FQH2mpfxw+fgJL//i8ty ++dg== X-Forwarded-Encrypted: i=1; AJvYcCXAQc4j9Og6QTkeZ2MhUUVX8W3Fh5/ipaXM8mUfur7X57RG48X3Xvlhf9wpuVWbhlpetpJE+15LbECmVqad@postgresql.org X-Gm-Message-State: AOJu0YwuOQN0iXYuPknAjN0iCOuIymLt+jBoStyNiZG+0pNfZvkD9sGl Rjc8vBDvbbNy5cYfHjyi7hqQA01ORhFNaqRd7WO0YOow/uLoktfXLdKRchmL8nNTsoabDLb0SNM A4LeV9/QTm1zXicKbIpbs5/lDuDk= X-Gm-Gg: ASbGncsjRlXKxLRs4FlGJneq1yb45GM9amICwR+EU5w+8xc8dGqdGjvUMqfVvi3wH8h gubYWorfVbqQ8Q5klnYfK8lY4hUSlbk2Pp+fIQfHV0u0Rko6bZH3ywcHgiCZtjvuEOqLzkmJ5OD TrBNnj1o8mH0RoZnjeXl0bgYGmi8wkLdFPXgg= X-Google-Smtp-Source: AGHT+IGkxWtYaiyij0y/RMxl9lOkmwaycONUEVfpJAgOlTTPUuzqlW3FwTEbo88ycih2e2PUnEAwlqRewYZyjCNMBks= X-Received: by 2002:a05:6871:29d:b0:2c2:2f9c:4dc7 with SMTP id 586e51a60fabf-2c78051a899mr3026156fac.35.1742572268652; Fri, 21 Mar 2025 08:51:08 -0700 (PDT) MIME-Version: 1.0 References: <88e3b55a-8ef8-4b53-8d71-6bfde1a07bc1@oss.nttdata.com> In-Reply-To: From: "David G. Johnston" Date: Fri, 21 Mar 2025 08:50:31 -0700 X-Gm-Features: AQ5f1JqsZkzwy5MtkgTDEsCNzUj2VeBctvPAmjnYY7qE_zDmfZhEkQA5muRARAc Message-ID: Subject: Re: Disabling vacuum truncate for autovacuum To: Nathan Bossart Cc: Fujii Masao , Robert Treat , Laurenz Albe , Gurjeet Singh , Andres Freund , Will Storey , Robert Haas , Postgres Hackers Content-Type: multipart/alternative; boundary="000000000000118f1b0630dc3971" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000118f1b0630dc3971 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Mar 20, 2025 at 2:31=E2=80=AFPM Nathan Bossart wrote: > On Thu, Mar 20, 2025 at 02:18:33PM -0700, David G. Johnston wrote: > > So my concern about dump/restore seems to be alleviated but then, why c= an > > we not just do whatever pg_dump is doing to decide whether the current > > value for vacuum_truncate is its default (and thus would not be dumped) > or > > not (and would be dumped)? > > pg_dump looks at the pg_class.reloptions array directly. In the vacuum > code, we look at the pre-parsed rd_options (see RelationParseRelOptions() > in relcache.c), which will have already resolved vacuum_truncate to its > default value if it was not explicitly set. We could probably look at > pg_class.reloptions directly in the vacuum code if we _really_ wanted to, > but I felt that putting this information into rd_options was much cleaner= . > > I understand this now and suggest the following comment changes based upon that understanding. diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index 645b5c0046..c795df6100 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -1912,7 +1912,9 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind) {"vacuum_index_cleanup", RELOPT_TYPE_ENUM, offsetof(StdRdOptions, vacuum_index_cleanup)}, {"vacuum_truncate", RELOPT_TYPE_BOOL, - offsetof(StdRdOptions, vacuum_truncate), offsetof(StdRdOptions, vacuum_truncate_set)}, + offsetof(StdRdOptions, vacuum_truncate), + /* vacuum_truncate uses the isset_offset member instead of a sentinel value */ + offsetof(StdRdOptions, vacuum_truncate_set)}, {"vacuum_max_eager_freeze_failure_rate", RELOPT_TYPE_REAL, offsetof(StdRdOptions, vacuum_max_eager_freeze_failure_rate)} }; diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h index 146aed47c2..fddeee0d54 100644 --- a/src/include/access/reloptions.h +++ b/src/include/access/reloptions.h @@ -152,7 +152,17 @@ typedef struct const char *optname; /* option's name */ relopt_type opttype; /* option's datatype */ int offset; /* offset of field in result struct */ - int isset_offset; /* if > 0, offset of "is set" field */ + /* + * The reloptions system knows whether a reloption has been set by the + * DBA or not. Historically, this information was lost when parsin= g + * the reloptions so we coped by using sentinel values. + * This doesn't work for boolean reloptions. For those, we + * need a place for the reloption parser to store its knowledge of + * whether the reloption was set. Set this field to an offset + * in the StdRdOptions struct. The pointed-to location needs to + * be a bool. A value of 0 here is interpreted as "no need to store". + */ + int isset_offset; } relopt_parse_elt; /* Local reloption definition */ diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index d94fddd7ce..461648aac6 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -344,7 +344,7 @@ typedef struct StdRdOptions int parallel_workers; /* max number of parallel workers */ StdRdOptIndexCleanup vacuum_index_cleanup; /* controls index vacuuming */ bool vacuum_truncate; /* enables vacuum to truncate a relation */ - bool vacuum_truncate_set; /* whether vacuum_truncate is set */ + bool vacuum_truncate_set; /* reserve space for isset status of vacuum_truncate */ /* * Fraction of pages in a relation that vacuum can eagerly scan and fail David J. --000000000000118f1b0630dc3971 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Thu, Mar 20, 2025 at 2:31=E2=80=AFPM Nathan Bossart <= ;nathandbossart@gmail.com&g= t; wrote:
On Thu, Mar 20, 2025 at= 02:18:33PM -0700, David G. Johnston wrote:
> So my concern about dump/restore seems to be alleviated but then, why = can
> we not just do whatever pg_dump is doing to decide whether the current=
> value for vacuum_truncate is its default (and thus would not be dumped= ) or
> not (and would be dumped)?

pg_dump looks at the pg_class.reloptions array directly.=C2=A0 In the vacuu= m
code, we look at the pre-parsed rd_options (see RelationParseRelOptions() in relcache.c), which will have already resolved vacuum_truncate to its
default value if it was not explicitly set.=C2=A0 We could probably look at=
pg_class.reloptions directly in the vacuum code if we _really_ wanted to, but I felt that putting this information into rd_options was much cleaner.<= br>

I understand this now and suggest the= following comment changes based=C2=A0upon that understanding.
=C2=A0diff --git a/src/backend/access/common/reloptions.c b/sr= c/backend/access/common/reloptions.c
index 645b5c0046..c795df6100 1006= 44
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/acc= ess/common/reloptions.c
@@ -1912,7 +1912,9 @@ default_reloptions(Datum r= eloptions, bool validate, relopt_kind kind)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 {"vacuum_index_cleanup", RELOPT_TYPE_= ENUM,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offsetof(S= tdRdOptions, vacuum_index_cleanup)},
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 {"vacuum_truncate", RELOPT_TYPE_BOOL,
- = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offsetof(StdRdOptions, vac= uum_truncate), offsetof(StdRdOptions, vacuum_truncate_set)},
+ =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offsetof(StdRdOptions, vacuum_tru= ncate),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* vacuum_tru= ncate uses the isset_offset member instead of a sentinel value */
+ =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offsetof(StdRdOptions, vacuum= _truncate_set)},
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= {"vacuum_max_eager_freeze_failure_rate", RELOPT_TYPE_REAL,
= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offsetof(StdRdOptio= ns, vacuum_max_eager_freeze_failure_rate)}
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }= ;
diff --git a/src/include/access/reloptions.h b/src/include/access/relo= ptions.h
index 146aed47c2..fddeee0d54 100644
--- a/src/include/access= /reloptions.h
+++ b/src/include/access/reloptions.h
@@ -152,7 +152,17= @@ typedef struct
=C2=A0 =C2=A0 =C2=A0 =C2=A0 const char *optname; =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* option's name */
=C2=A0 =C2= =A0 =C2=A0 =C2=A0 relopt_type opttype; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0/* option's datatype */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 int =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offset; =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* offset of field in = result struct */
- =C2=A0 =C2=A0 =C2=A0 int =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 isset_offset; =C2=A0 /* if > 0= , offset of "is set" field */
+ =C2=A0 =C2=A0 =C2=A0 /*
+ = =C2=A0 =C2=A0 =C2=A0 =C2=A0* The reloptions system knows whether a reloptio= n has been set by the
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0* DBA or not.=C2=A0 H= istorically, this information was lost when parsing
+ =C2=A0 =C2=A0 =C2= =A0 =C2=A0* the reloptions so we coped by using sentinel values.
+ =C2= =A0 =C2=A0 =C2=A0 =C2=A0* This doesn't work for boolean reloptions.=C2= =A0 For those, we
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0* need a place for the re= loption parser to store its knowledge of
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0* = whether the reloption was set.=C2=A0 Set this field to an offset
+ =C2= =A0 =C2=A0 =C2=A0 =C2=A0* in the StdRdOptions struct.=C2=A0 The pointed-to = location needs to
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0* be a bool.=C2=A0 A valu= e of 0 here is interpreted as "no need to store".
+ =C2=A0 =C2= =A0 =C2=A0 =C2=A0*/
+ =C2=A0 =C2=A0 =C2=A0 int =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 isset_offset;
=C2=A0} relo= pt_parse_elt;
=C2=A0
=C2=A0/* Local reloption definition */
diff -= -git a/src/include/utils/rel.h b/src/include/utils/rel.h
index d94fddd7c= e..461648aac6 100644
--- a/src/include/utils/rel.h
+++ b/src/include/= utils/rel.h
@@ -344,7 +344,7 @@ typedef struct StdRdOptions
=C2=A0 = =C2=A0 =C2=A0 =C2=A0 int =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 parallel_workers; =C2=A0 =C2=A0 =C2=A0 /* max number o= f parallel workers */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 StdRdOptIndexCleanup v= acuum_index_cleanup; =C2=A0 =C2=A0 =C2=A0/* controls index vacuuming */
= =C2=A0 =C2=A0 =C2=A0 =C2=A0 bool =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0v= acuum_truncate; =C2=A0 =C2=A0 =C2=A0 =C2=A0/* enables vacuum to truncate a = relation */
- =C2=A0 =C2=A0 =C2=A0 bool =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0vacuum_truncate_set; =C2=A0 =C2=A0/* whether vacuum_truncate is s= et */
+ =C2=A0 =C2=A0 =C2=A0 bool =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0vacuum_truncate_set; =C2=A0 =C2=A0/* reserve space for isset status of v= acuum_truncate */
=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0 /*
=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0* Fraction of pages in a relation that vacuum can e= agerly scan and fail
=
David J.


--000000000000118f1b0630dc3971--