public inbox for [email protected]
help / color / mirror / Atom feedRe: Should vacuum process config file reload more often
6+ messages / 3 participants
[nested] [flat]
* Re: Should vacuum process config file reload more often
@ 2023-04-17 02:04 Masahiko Sawada <[email protected]>
2023-04-25 12:39 ` Re: Should vacuum process config file reload more often Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Masahiko Sawada @ 2023-04-17 02:04 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Melanie Plageman <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers; Amit Kapila <[email protected]>
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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Should vacuum process config file reload more often
2023-04-17 02:04 Re: Should vacuum process config file reload more often Masahiko Sawada <[email protected]>
@ 2023-04-25 12:39 ` Daniel Gustafsson <[email protected]>
2023-04-25 13:31 ` Re: Should vacuum process config file reload more often Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Gustafsson @ 2023-04-25 12:39 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: Melanie Plageman <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers; Amit Kapila <[email protected]>
> On 17 Apr 2023, at 04:04, Masahiko Sawada <[email protected]> wrote:
> I've attached an updated patch for fixing at_dobalance condition.
I revisited this and pushed it to all supported branches after another round of
testing and reading.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Should vacuum process config file reload more often
2023-04-17 02:04 Re: Should vacuum process config file reload more often Masahiko Sawada <[email protected]>
2023-04-25 12:39 ` Re: Should vacuum process config file reload more often Daniel Gustafsson <[email protected]>
@ 2023-04-25 13:31 ` Masahiko Sawada <[email protected]>
2023-04-25 13:35 ` Re: Should vacuum process config file reload more often Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Masahiko Sawada @ 2023-04-25 13:31 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Melanie Plageman <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers; Amit Kapila <[email protected]>
On Tue, Apr 25, 2023 at 9:39 PM Daniel Gustafsson <[email protected]> wrote:
>
> > On 17 Apr 2023, at 04:04, Masahiko Sawada <[email protected]> wrote:
>
> > I've attached an updated patch for fixing at_dobalance condition.
>
> I revisited this and pushed it to all supported branches after another round of
> testing and reading.
Thanks!
Can we mark the open item "Can't disable autovacuum cost delay through
storage parameter" as resolved?
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Should vacuum process config file reload more often
2023-04-17 02:04 Re: Should vacuum process config file reload more often Masahiko Sawada <[email protected]>
2023-04-25 12:39 ` Re: Should vacuum process config file reload more often Daniel Gustafsson <[email protected]>
2023-04-25 13:31 ` Re: Should vacuum process config file reload more often Masahiko Sawada <[email protected]>
@ 2023-04-25 13:35 ` Daniel Gustafsson <[email protected]>
2023-04-25 15:02 ` Re: Should vacuum process config file reload more often Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Gustafsson @ 2023-04-25 13:35 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: Melanie Plageman <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers; Amit Kapila <[email protected]>
> On 25 Apr 2023, at 15:31, Masahiko Sawada <[email protected]> wrote:
>
> On Tue, Apr 25, 2023 at 9:39 PM Daniel Gustafsson <[email protected]> wrote:
>>
>>> On 17 Apr 2023, at 04:04, Masahiko Sawada <[email protected]> wrote:
>>
>>> I've attached an updated patch for fixing at_dobalance condition.
>>
>> I revisited this and pushed it to all supported branches after another round of
>> testing and reading.
>
> Thanks!
>
> Can we mark the open item "Can't disable autovacuum cost delay through
> storage parameter" as resolved?
Yes, I've gone ahead and done that now.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Should vacuum process config file reload more often
2023-04-17 02:04 Re: Should vacuum process config file reload more often Masahiko Sawada <[email protected]>
2023-04-25 12:39 ` Re: Should vacuum process config file reload more often Daniel Gustafsson <[email protected]>
2023-04-25 13:31 ` Re: Should vacuum process config file reload more often Masahiko Sawada <[email protected]>
2023-04-25 13:35 ` Re: Should vacuum process config file reload more often Daniel Gustafsson <[email protected]>
@ 2023-04-25 15:02 ` Masahiko Sawada <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Masahiko Sawada @ 2023-04-25 15:02 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Melanie Plageman <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers; Amit Kapila <[email protected]>
On Tue, Apr 25, 2023 at 10:35 PM Daniel Gustafsson <[email protected]> wrote:
>
> > On 25 Apr 2023, at 15:31, Masahiko Sawada <[email protected]> wrote:
> >
> > On Tue, Apr 25, 2023 at 9:39 PM Daniel Gustafsson <[email protected]> wrote:
> >>
> >>> On 17 Apr 2023, at 04:04, Masahiko Sawada <[email protected]> wrote:
> >>
> >>> I've attached an updated patch for fixing at_dobalance condition.
> >>
> >> I revisited this and pushed it to all supported branches after another round of
> >> testing and reading.
> >
> > Thanks!
> >
> > Can we mark the open item "Can't disable autovacuum cost delay through
> > storage parameter" as resolved?
>
> Yes, I've gone ahead and done that now.
Great, thank you!
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
@ 2026-07-10 04:36 Rui Zhao <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Rui Zhao @ 2026-07-10 04:36 UTC (permalink / raw)
To: Akshay Joshi <[email protected]>; +Cc: Marcos Pegoraro <[email protected]>; Zsolt Parragi <[email protected]>; [email protected]
Hi Akshay,
Thanks for v19. I re-ran the pg_dump round-trip check (the 002_pg_dump.pl
scenarios plus the whole regression database) on current master (2e6578292a).
The five fixes hold up: SET STORAGE/COMPRESSION now use ONLY, invalid indexes
are skipped, SET STATISTICS / CLUSTER ON / DISABLE RULE are emitted, and the
child-default and named-NOT NULL cases I sent all round-trip now. 494 of 562
tables match pg_dump exactly (was 483/560 on v17). Three things came out of
the run below; the third is a side effect of that invalid-index skip.
1) The named-NOT NULL handling for inheritance/partition children still isn't
complete. It works when the child constraint has a user-defined name, but the
corpus turns up two ways it still goes wrong.
1a) For an auto-named inherited NOT NULL it emits an out-of-line ADD CONSTRAINT
that collides with the one INHERITS/PARTITION OF already propagates, giving
non-replayable DDL:
CREATE TABLE nt (a int PRIMARY KEY);
CREATE TABLE ntc (PRIMARY KEY (a) DEFERRABLE) INHERITS (nt);
SELECT d FROM pg_get_table_ddl('ntc'::regclass, owner => false) d;
-- CREATE TABLE public.ntc () INHERITS (public.nt);
-- ALTER TABLE public.ntc ADD CONSTRAINT ntc_a_not_null NOT NULL a;
-- ALTER TABLE public.ntc ADD CONSTRAINT ntc_pkey PRIMARY KEY (a)
DEFERRABLE;
-- replaying that, with nt already restored first so INHERITS propagates
-- its nt_a_not_null down to ntc:
ERROR: cannot create not-null constraint "ntc_a_not_null" on
column "a" of table "ntc"
DETAIL: A not-null constraint named "nt_a_not_null" already
exists for this column.
(notnull_tbl4_cld2 in the regression suite fails the same way.)
1b) Even where it does replay, a partition attached from a standalone table
keeps its name but comes back with the wrong constraint locality, because the
constraint is folded into PARTITION OF instead of a standalone CREATE + ATTACH:
CREATE TABLE mp (a int NOT NULL) PARTITION BY LIST (a);
CREATE TABLE mp1 (a int CONSTRAINT mp1_nn NOT NULL);
ALTER TABLE mp ATTACH PARTITION mp1 FOR VALUES IN (1);
-- source mp1.mp1_nn: conislocal = false (the ATTACH merged it
into the parent's)
SELECT d FROM pg_get_table_ddl('mp'::regclass, owner => false) d;
-- CREATE TABLE public.mp (a integer NOT NULL) PARTITION BY LIST (a);
-- CREATE TABLE public.mp1 PARTITION OF public.mp (CONSTRAINT
mp1_nn NOT NULL a) FOR VALUES IN (1);
Declaring the constraint inline makes it local, so mp1.mp1_nn comes back with
conislocal = true, and the behavior diverges once the parent's NOT NULL is
dropped:
ALTER TABLE mp ALTER a DROP NOT NULL;
-- source: mp1.a is no longer NOT NULL; reconstructed: mp1.a stays NOT NULL
pg_dump reconstructs both faithfully. For reference, this is where it puts
each constraint:
- In the CREATE TABLE body: NOT NULL, and validated CHECK.
- As a separate ALTER TABLE ... ADD CONSTRAINT: PRIMARY KEY, UNIQUE, EXCLUDE,
FOREIGN KEY, and NOT VALID CHECK.
The split is the same on plain tables, inheritance children and partitions;
only the table shape differs:
- A plain-inheritance child keeps CREATE ... INHERITS and emits only its
locally-owned constraints (purely-inherited NOT NULL/CHECK are left to
INHERITS); the NOT NULL goes in the body unnamed so it auto-names, e.g.
for the 1a case: CREATE TABLE ntc (NOT NULL a) INHERITS (nt); ...
- A partition is never dumped with CREATE ... PARTITION OF; it is a standalone
CREATE TABLE plus ALTER TABLE ONLY parent ATTACH PARTITION, so the child's
column order, constraint names and conislocal come from its own definition
plus the merge ATTACH performs.
That partition path is the same standalone+ATTACH that the reordered-column
case you deferred needs, so 1b likely folds into that follow-up. I'll
leave the exact approach to you.
2) A NOT VALID CHECK constraint loses its NOT VALID flag on round-trip. The
constraint is emitted inline in CREATE TABLE, where it is validated against the
(empty) table, so the reconstructed constraint is marked validated:
CREATE TABLE nv (a int);
ALTER TABLE nv ADD CONSTRAINT chk CHECK (a < 50) NOT VALID;
SELECT d FROM pg_get_table_ddl('nv'::regclass, owner => false) d;
-- CREATE TABLE public.nv (a integer, CONSTRAINT chk CHECK ((a <
50)) NOT VALID);
-- source nv.chk: convalidated = false
-- reconstructed nv.chk: convalidated = true
pg_dump emits NOT VALID constraints as a separate ALTER TABLE ... ADD
CONSTRAINT ... NOT VALID for this reason.
3) The invalid-index skip added for the earlier finding also drops legitimate
partitioned indexes. emit_indexes now has:
/* Skip invalid indexes; they may be left over from a failed
CREATE INDEX CONCURRENTLY. */
if (!idxform->indisvalid)
{
ReleaseSysCache(indTup);
continue;
}
but a partitioned index on the parent is normally indisvalid = false (e.g. one
created with ON ONLY, or before all child indexes are attached), so the whole
index is lost:
CREATE TABLE p (a int) PARTITION BY RANGE (a);
CREATE TABLE p1 PARTITION OF p FOR VALUES FROM (0) TO (100);
CREATE INDEX ON ONLY p (a); -- p_a_idx.indisvalid = false
SELECT d FROM pg_get_table_ddl('p'::regclass, owner => false) d;
-- CREATE TABLE public.p (a integer) PARTITION BY RANGE (a);
-- CREATE TABLE public.p1 PARTITION OF public.p FOR VALUES FROM
(0) TO (100);
No CREATE INDEX at all; pg_dump keeps it as "CREATE INDEX p_a_idx ON ONLY
public.p USING btree (a)". That's the pg_dump condition I quoted earlier --
"i.indisvalid OR t2.relkind = 'p'"; the relkind = 'p' half needs to stay so
partitioned indexes aren't filtered out along with the failed-CIC ones.
Thanks,
Rui
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-07-10 04:36 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-04-17 02:04 Re: Should vacuum process config file reload more often Masahiko Sawada <[email protected]>
2023-04-25 12:39 ` Daniel Gustafsson <[email protected]>
2023-04-25 13:31 ` Masahiko Sawada <[email protected]>
2023-04-25 13:35 ` Daniel Gustafsson <[email protected]>
2023-04-25 15:02 ` Masahiko Sawada <[email protected]>
2026-07-10 04:36 Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements Rui Zhao <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox