Re: Should vacuum process config file reload more often

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, Melanie Plageman <melanieplageman(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: Re: Should vacuum process config file reload more often
Date: 2023-04-27 12:54:57
Message-ID: E30A005D-0CD4-40E2-8865-B1AC85A1AD71@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 27 Apr 2023, at 14:10, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Thu, Apr 27, 2023 at 6:30 PM John Naylor
> <john(dot)naylor(at)enterprisedb(dot)com> wrote:
>>
>>
>> On Fri, Apr 7, 2023 at 6:08 AM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>>>
>>> I had another read-through and test-through of this version, and have applied
>>> it with some minor changes to comments and whitespace. Thanks for the quick
>>> turnaround times on reviews in this thread!
>>
>> - VacuumFailsafeActive = false;
>> + Assert(!VacuumFailsafeActive);
>>
>> I can trigger this assert added in commit 7d71d3dd08.
>>
>> First build with the patch in [1], then:
>>
>> session 1:
>>
>> CREATE EXTENSION xid_wraparound ;
>>
>> CREATE TABLE autovacuum_disabled(id serial primary key, data text) WITH (autovacuum_enabled=false);
>> INSERT INTO autovacuum_disabled(data) SELECT generate_series(1,1000);
>>
>> -- I can trigger without this, but just make sure it doesn't get vacuumed
>> BEGIN;
>> DELETE FROM autovacuum_disabled WHERE id % 2 = 0;
>>
>> session 2:
>>
>> -- get to failsafe limit
>> SELECT consume_xids(1*1000*1000*1000);
>> INSERT INTO autovacuum_disabled(data) SELECT 1;
>> SELECT consume_xids(1*1000*1000*1000);
>> INSERT INTO autovacuum_disabled(data) SELECT 1;
>>
>> VACUUM autovacuum_disabled;
>>
>> WARNING: cutoff for removing and freezing tuples is far in the past
>> HINT: Close open transactions soon to avoid wraparound problems.
>> You might also need to commit or roll back old prepared transactions, or drop stale replication slots.
>> WARNING: bypassing nonessential maintenance of table "john.public.autovacuum_disabled" as a failsafe after 0 index scans
>> DETAIL: The table's relfrozenxid or relminmxid is too far in the past.
>> HINT: Consider increasing configuration parameter "maintenance_work_mem" or "autovacuum_work_mem".
>> You might also need to consider other ways for VACUUM to keep up with the allocation of transaction IDs.
>> server closed the connection unexpectedly
>>
>> #0 0x00007ff31f68ebec in __pthread_kill_implementation ()
>> from /lib64/libc.so.6
>> #1 0x00007ff31f63e956 in raise () from /lib64/libc.so.6
>> #2 0x00007ff31f6287f4 in abort () from /lib64/libc.so.6
>> #3 0x0000000000978032 in ExceptionalCondition (
>> conditionName=conditionName(at)entry=0xa4e970 "!VacuumFailsafeActive",
>> fileName=fileName(at)entry=0xa4da38 "../src/backend/access/heap/vacuumlazy.c", lineNumber=lineNumber(at)entry=392) at ../src/backend/utils/error/assert.c:66
>> #4 0x000000000058c598 in heap_vacuum_rel (rel=0x7ff31d8a97d0,
>> params=<optimized out>, bstrategy=<optimized out>)
>> at ../src/backend/access/heap/vacuumlazy.c:392
>> #5 0x000000000069af1f in table_relation_vacuum (bstrategy=0x14ddca8,
>> params=0x7ffec28585f0, rel=0x7ff31d8a97d0)
>> at ../src/include/access/tableam.h:1705
>> #6 vacuum_rel (relid=relid(at)entry=16402, relation=relation(at)entry=0x0,
>> params=params(at)entry=0x7ffec28585f0, skip_privs=skip_privs(at)entry=true,
>> bstrategy=bstrategy(at)entry=0x14ddca8)
>> at ../src/backend/commands/vacuum.c:2202
>> #7 0x000000000069b0e4 in vacuum_rel (relid=16398, relation=<optimized out>,
>> params=params(at)entry=0x7ffec2858850, skip_privs=skip_privs(at)entry=false,
>> bstrategy=bstrategy(at)entry=0x14ddca8)
>> at ../src/backend/commands/vacuum.c:2236
>
> Good catch. I think the problem is that vacuum_rel() is called
> recursively and we don't reset VacuumFailsafeActive before vacuuming
> the toast table. I think we should reset it in heap_vacuum_rel()
> instead of Assert(). It's possible that we trigger the failsafe mode
> only for either one.Please find the attached patch.

Agreed, that matches my research and testing, I have the same diff here and it
passes testing and works as intended. This was briefly discussed in [0] and
slightly upthread from there but then missed. I will do some more looking and
testing but I'm fairly sure this is the right fix, so unless I find something
else I will go ahead with this.

xid_wraparound is a really nifty testing tool. Very cool.

--
Daniel Gustafsson

[0] CAAKRu_b1HjGCTsFpUnmwLNS8NeXJ+JnrDLhT1osP+Gq9HCU+Rw(at)mail(dot)gmail(dot)com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2023-04-27 13:53:17 Re: Fix a test case in 035_standby_logical_decoding.pl
Previous Message Jakub Wartak 2023-04-27 12:47:41 Re: In-placre persistance change of a relation