public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Geoghegan <[email protected]>
To: Andres Freund <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations
Date: Wed, 30 Mar 2022 19:37:58 -0700
Message-ID: <CAH2-WzmRZEzeGvLv8yDW0AbFmSvJjTziORqjVUrf74mL4GL0Ww@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAH2-WzmL98Ayv+5MsUF1xy_+brhconV43QgDi=PTBO5GWoioEg@mail.gmail.com>
<CA+TgmoZiSOY6H7aadw5ZZGm7zYmfDzL6nwmL5V7GL4HgJgLF_w@mail.gmail.com>
<CAH2-Wz=D3BzDE=ygzEb6AYkr9YzuZ+D1NpC=L9CCPG-sZVz_LA@mail.gmail.com>
<CAH2-Wzk3fNBa_S3Ngi+16GQiyJ=AmUu3oUY99syMDTMRxitfyQ@mail.gmail.com>
<CA+TgmoYvqfhtfobZHJb-LhNy6QCrD9sqE9gtRgntFJFkmo2_XQ@mail.gmail.com>
<CAH2-Wz=HTfWpmeXeCP-Q9CuWGbHeZW-j75tgtdL7B7e2O2TnBA@mail.gmail.com>
<CAH2-Wz=Q2NQprVdf8wojerm-WYwy_eKmLB64mwJ25Prjf=TdvQ@mail.gmail.com>
<[email protected]>
<CAH2-WzmwM81qMxjYdm4-Z51PKwsgXXAr21bfBRncXCR7VB8W1g@mail.gmail.com>
<CAH2-Wzn_LLtTk-jGranhLcciSt_70pFKJNngZOf8Gt_MNgTmnA@mail.gmail.com>
<[email protected]>
On Wed, Mar 30, 2022 at 7:00 PM Andres Freund <[email protected]> wrote:
> Something vaguely like EXPECT_EQ_U32 in regress.c. Maybe
> AssertCmp(type, a, op, b),
>
> Then the assertion could have been something like
> AssertCmp(int32, diff, >, 0)
I'd definitely use them if they were there.
> Does the line number in the failed run actually correspond to the xid, rather
> than the mxid case? I didn't check.
Yes, I verified -- definitely relfrozenxid.
> You can do something similar locally on linux with
> make -Otarget -C contrib/ -j48 -s USE_MODULE_DB=1 installcheck prove_installcheck=true
> (the prove_installcheck=true to prevent tap tests from running, we don't seem
> to have another way for that)
>
> I don't think windows uses USE_MODULE_DB=1, but it allows to cause a lot more
> load concurrently than running tests serially...
Can't get it to fail locally with that recipe.
> > Assert(vacrel->NewRelfrozenXid == OldestXmin ||
> > TransactionIdPrecedesOrEquals(vacrel->relfrozenxid,
> > vacrel->NewRelfrozenXid));
>
> The comment in your patch says "is either older or newer than FreezeLimit" - I
> assume that's some rephrasing damage?
Both the comment and the assertion are correct. I see what you mean, though.
> Perhaps it's worth commiting improved assertions on master? If this is indeed
> a pre-existing bug, and we're just missing due to slightly less stringent
> asserts, we could rectify that separately.
I don't think there's much chance of the assertion actually hitting
without the rest of the patch series. The new relfrozenxid value is
always going to be OldestXmin - vacuum_min_freeze_age on HEAD, while
with the patch it's sometimes close to OldestXmin. Especially when you
have lots of dead tuples that you churn through constantly (like
pgbench_tellers, or like these system catalogs on the CI test
machine).
> Hm. This triggers some vague memories. There's some oddities around shared
> relations being vacuumed separately in all the databases and thus having
> separate horizons.
That's what I was thinking of, obviously.
> After "remembering" that, I looked in the cirrus log for the failed run, and
> the worker was processing shared a shared relation last:
>
> 2022-03-30 03:48:30.238 GMT [5984][autovacuum worker] LOG: automatic analyze of table "contrib_regression.pg_catalog.pg_authid"
I noticed the same thing myself. Should have said sooner.
> Perhaps this ought to be an elog() instead of an Assert()? Something has gone
> pear shaped if we get here... It's a bit annoying though, because it'd have to
> be a PANIC to be visible on the bf / CI :(.
Yeah, a WARNING would be good here. I can write a new version of my
patch series with a separation patch for that this evening. Actually,
better make it a PANIC for now...
--
Peter Geoghegan
view thread (72+ 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]
Subject: Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations
In-Reply-To: <CAH2-WzmRZEzeGvLv8yDW0AbFmSvJjTziORqjVUrf74mL4GL0Ww@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