public inbox for [email protected]
help / color / mirror / Atom feedFrom: David G. Johnston <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: Robert Treat <[email protected]>
Cc: Laurenz Albe <[email protected]>
Cc: Gurjeet Singh <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Will Storey <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Postgres Hackers <[email protected]>
Subject: Re: Disabling vacuum truncate for autovacuum
Date: Fri, 21 Mar 2025 08:50:31 -0700
Message-ID: <CAKFQuwbtKSAJv=BPmQOFv-fwPH2p1HzBH54EDBvB0ay9sHX-cw@mail.gmail.com> (raw)
In-Reply-To: <Z9yJNZ1C4TJfU-nY@nathan>
References: <CABV9wwNagobmRMCxs0dKqWmKXaQTUQj+oOOTw=fsPAgbBfVhew@mail.gmail.com>
<Z9g8a4HGLUcSo96v@nathan>
<Z9ohFZtDaZnkch7u@nathan>
<[email protected]>
<Z9rlb0jgNLm8ULAB@nathan>
<Z9rsMX5ZOL5E0cQy@nathan>
<Z9wxuP1AY303wAxM@nathan>
<CAKFQuwYKtEUYKS+18gRs-xPhn0qOJgM2KGyyWVCODHuVn9F-XQ@mail.gmail.com>
<Z9xaroJSrS44HyKG@nathan>
<CAKFQuwZJtw7Ojr=Z0xgA0G7BW6qSDkb0fMKgyaQULdOxHg_chA@mail.gmail.com>
<Z9yJNZ1C4TJfU-nY@nathan>
On Thu, Mar 20, 2025 at 2:31 PM Nathan Bossart <[email protected]>
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. 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 parsing
+ * 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.
view thread (5+ 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], [email protected], [email protected]
Subject: Re: Disabling vacuum truncate for autovacuum
In-Reply-To: <CAKFQuwbtKSAJv=BPmQOFv-fwPH2p1HzBH54EDBvB0ay9sHX-cw@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