From: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> |
---|---|
To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
Cc: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Robert Treat <rob(at)xzilla(dot)net>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Gurjeet Singh <gurjeet(at)singh(dot)im>, Andres Freund <andres(at)anarazel(dot)de>, Will Storey <will(at)summercat(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Postgres Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Disabling vacuum truncate for autovacuum |
Date: | 2025-03-21 15:50:31 |
Message-ID: | CAKFQuwbtKSAJv=BPmQOFv-fwPH2p1HzBH54EDBvB0ay9sHX-cw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-general pgsql-hackers |
On Thu, Mar 20, 2025 at 2:31 PM Nathan Bossart <nathandbossart(at)gmail(dot)com>
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.
From | Date | Subject | |
---|---|---|---|
Next Message | David G. Johnston | 2025-03-21 15:54:55 | Re: Disabling vacuum truncate for autovacuum |
Previous Message | David G. Johnston | 2025-03-21 15:14:14 | Re: After upgrading libpq, the same function(PQftype) call returns a different OID |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2025-03-21 15:52:29 | Re: Parallel safety for extern params |
Previous Message | Peter Geoghegan | 2025-03-21 15:36:18 | Re: Adding skip scan (including MDAM style range skip scan) to nbtree |