public inbox for [email protected]  
help / color / mirror / Atom feed
From: Daniel Gustafsson <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: John Naylor <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Amit Kapila <[email protected]>
Subject: Re: Should vacuum process config file reload more often
Date: Thu, 27 Apr 2023 23:25:30 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAKRu_Z8+UqT8D=k-bVuE8525Z3ri4_xfCuOVnuQ-5yWc+tUew@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CAAKRu_Yg4-kWH_EN7wtpm3z9B+H27UTmOxzzYv1Vg=YdZ6v_rw@mail.gmail.com>
	<CAD21AoCGdxYAJHmE_nXw_MFw1+Qtn3vcThRu5FiadukgQnpWeQ@mail.gmail.com>
	<[email protected]>
	<CAAKRu_aoa0YT36q2n3xa9MZYdepeH4Jyf2EVJj=7YdC_1c974Q@mail.gmail.com>
	<CAAKRu_bSsChKsyEy1ZV2XJQ22FoDdA56efjNr9==m2TO94557w@mail.gmail.com>
	<CAD21AoDi4i4tj1Cf9BG0q2iyVhHisQ2xxVBZajEqExX6t6_b3g@mail.gmail.com>
	<CAAKRu_bkg+0WAGTxsy0HP75RzFbPwqz3ctYq1kThjFhSL69upw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAAKRu_ap7nhmqtKQ1TRo3LzHG3FK=YpGO=O0019M3BbN7ysm3A@mail.gmail.com>
	<[email protected]>
	<CAAKRu_b1HjGCTsFpUnmwLNS8NeXJ+JnrDLhT1osP+Gq9HCU+Rw@mail.gmail.com>
	<[email protected]>
	<CAAKRu_bJ2DJxYoMkyMo9UpoX74LY5d2Jir86QWFUftrGKWP5Bw@mail.gmail.com>
	<CA+TgmoYDYHht9zsHye3tywx7oWGMP4s_6AGefHkDsMuGHLzT7w@mail.gmail.com>
	<CAAKRu_auVL_uE794zwiX6t18LJvKtJ5mrmV9tkbpWzS8AYeChQ@mail.gmail.com>
	<CAAKRu_aX+MLn32kPnAQ30WChbiLTkKKmY9e8+B1ZY1BokpQZYA@mail.gmail.com>
	<CAAKRu_aH-UbvNCQq-EouNUTjtGexQTRVo7ik_vYR4WUocVny7w@mail.gmail.com>
	<CA+TgmoY3EaCz3_7F3+Sb3FUHonf4nxdC-_UWnn6fWEPtXU7t5g@mail.gmail.com>
	<[email protected]>
	<CAAKRu_YmQg_i=0DBfPzy1MMc2GjfuJFo=0rFv4fCzRQStLhjvw@mail.gmail.com>
	<CAAKRu_beQECOV4WfU6nG0hhEVobCnH88Y8MQi1LrTwMfRyxuCw@mail.gmail.com>
	<[email protected]>
	<CAAKRu_Znk6WCCQqEw8hv3+X1Fm8gG0x4KKRAkKmO2JOcA-77-g@mail.gmail.com>
	<[email protected]>
	<CAFBsxsFz=GqaG5Ens5aNgVYoV2Y+pfMUijX0ku+CCkWfALwiqg@mail.gmail.com>
	<CAD21AoB_NQwdznBqcTMPv=vx71feDvBTNEMAvaUMAGqNcruk9Q@mail.gmail.com>
	<[email protected]>
	<CAAKRu_Z8+UqT8D=k-bVuE8525Z3ri4_xfCuOVnuQ-5yWc+tUew@mail.gmail.com>

> On 27 Apr 2023, at 16:53, Melanie Plageman <[email protected]> wrote:
> On Thu, Apr 27, 2023 at 8:55 AM Daniel Gustafsson <[email protected]> wrote:
>> 
>>> On 27 Apr 2023, at 14:10, Masahiko Sawada <[email protected]> wrote:

>>> 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.Makes sense to me too.
> 
> Fix LGTM.

Thanks for review. I plan to push this in the morning.

> Though we previously set it to false before this series of patches,
> perhaps it is
> worth adding a comment about why VacuumFailsafeActive must be reset here
> even though we reset it before vacuuming each table?

Agreed. 

--
Daniel Gustafsson







view thread (26+ 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]
  Subject: Re: Should vacuum process config file reload more often
  In-Reply-To: <[email protected]>

* 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