Date: Tue, 12 May 2026 18:47:21 +0900
From: Yugo Nagata <nagata@sraoss.co.jp>
To: Sami Imseih <samimseih@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Pgsql Hackers
 <pgsql-hackers@postgresql.org>
Subject: Re: Track skipped tables during autovacuum and autoanalyze
Message-Id: <20260512184721.bf257567ab52d7d48a962350@sraoss.co.jp>
In-Reply-To: 
 <CAA5RZ0vwgXZ5kF2GvYBR+Ma1LPSbDjE9pjANzSiUw3wmpv51PQ@mail.gmail.com>
References: <20260324151133.7940a5c1f2ebd594d54da481@sraoss.co.jp>
	<CAA5RZ0snnePNW1NKGKh+NyJ1CY26T5F_6-tTq+BHWM2kj1fN1g@mail.gmail.com>
	<20260325012847.e026ba1860c07288efe3e97d@sraoss.co.jp>
	<20260326192203.e6dbb8d80f8d27dc15ceee59@sraoss.co.jp>
	<acW8MW-Gb8Aj_TH2@paquier.xyz>
	<20260327163549.b5df519c0099970ddbb3412d@sraoss.co.jp>
	<CAA5RZ0tNh03LLgJVMA=PXSdY8YVoui4_GyfbfTrYK5cka3Q9Rw@mail.gmail.com>
	<20260328161802.f35b5a3e739566ffd7c1053b@sraoss.co.jp>
	<20260413170551.5ec43ba5a2c848f0d46c6a0b@sraoss.co.jp>
	<CAA5RZ0tUHU-r=Tc47P2DZyytF7x5h5OwBiRABe_dZt+zWNqe9g@mail.gmail.com>
	<20260427203207.32aa6ca37f2a18a05508dfda@sraoss.co.jp>
	<CAA5RZ0vwgXZ5kF2GvYBR+Ma1LPSbDjE9pjANzSiUw3wmpv51PQ@mail.gmail.com>
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?OcX6UOMkqaK2kXth0nAvDdyFH9QWi7bPwFwIl3f1OXa1eSqh8xedMvZ9+pF2?=
 =?us-ascii?Q?iu8tedI4P5pJJ4s4R8dO30umSr9Sey25Vn4qJ8dCfJN6WrFMbP9iDWfrjSut?=
 =?us-ascii?Q?jy4uSxfxySkY9rLplrEc78kutFQbXwfkGCbQ68GsUPSMj/OfbaDXDRoRubA0?=
 =?us-ascii?Q?cGn0nNVWobz/VsPavePN3K6/AF6pkuugVDfrJulmdxCke5mYDBiXfqhacxd7?=
 =?us-ascii?Q?0C/fB5CbfIdKNamZqFyHSML3cBn0P7fYqAnuwMyVgXR46/5YI82/55JuwGr2?=
 =?us-ascii?Q?gyRIP74z2Yw2/03KwwO+lAaVymC7Gbiil1F6XFz0APmKh3bg0K6Nn9b53Gka?=
 =?us-ascii?Q?8HB0VhKv9Dy6e4uAO0eVfdneIO2tIIJ+55OKhwGcCdy9hyg+0Hxebqcu6oIC?=
 =?us-ascii?Q?kyew2KA/Gg2/CSo3k0W6ptnoVHJNyHuuz6Mz4WOT7zaY3kiCVrMWeeI2kBlS?=
 =?us-ascii?Q?ZrrupAr1lP6t5SudgwmDXSXFh6DmVYf9ZypoLvpQOQlVwu6iyoY0nisGyoTJ?=
 =?us-ascii?Q?ToF60cBdHJd8aCEoXxdtCcwgM83CjTWudEiJ8tm58XYuqNt+STbfzOWrXUOM?=
 =?us-ascii?Q?Fe5+iwX8rkfAobMuvXR+isUoDia3S67ZilALGaYM9Qz5Swxn+eGFDMszcW+7?=
 =?us-ascii?Q?7+M0r1vLHssI3rDV/vaHwssXnJwYHk0MhWXTzhyH5cXq2qm0F1wtVOtAEP8X?=
 =?us-ascii?Q?bFpjpJk2fU7rOOS6KLTb1WE0iuLJC7rPhyCqUweQjaI1XlmMkdnPk1YIbe3O?=
 =?us-ascii?Q?cics1H4szho8Hghls7N0o8S3Dc8czwsEQEtXlAIILyKmdTA5bBTeprg6N11H?=
 =?us-ascii?Q?aUhoUOaS6htQUrCNzPGk0y3hVNmHGLqbdrYXOtUc2WKwi1feHnY5GfW3tKqg?=
 =?us-ascii?Q?yA3D/mCimCePFqnbhV5loATNYupeSWSc9DHNHSDEGNL+W+cXNXGWQE9ATBRC?=
 =?us-ascii?Q?tWim5q8Vp/bIhAuz+pQ+0Xtn/TFnxWpPWM8kIA6+l6nt/ez64Qay6Hugf3NW?=
 =?us-ascii?Q?yWw3Lw3ERksoyewCjj/Uj3r3RjL/zWqRH8qTEYwW7JrK8YbDKAkX3ZtQHD1+?=
 =?us-ascii?Q?XsbetB0CWZTGtJ70sCLO75ehwlsku0W1WwhNEtpcl80mzHMB6PbZ7JQ+/Jxk?=
 =?us-ascii?Q?Nq5aMfLEySn/nbR1v/Y9WVuOTqPaRV4AC4HSlkwibUqncCZ5AQAVda1LBX/G?=
 =?us-ascii?Q?npvZuzKvqYYJvwfcMKY5Zy3aqLpjQCd8XIH1D5jr1k2W1+2avEPJpM3Q+R0b?=
 =?us-ascii?Q?v2jtLiMsgAcikHMNJs29ipnldsJXDRlQmRaDxKrcP/ZJQosJXO2bp3xpgWHY?=
 =?us-ascii?Q?ldp8iVpIyj6AxmYnIZ+TUEougksGdHUECsF5C2al1GrjHiIVGnXKcwj268Mz?=
 =?us-ascii?Q?BwAUsNHF7sCV3czx69XKQsopUg7li00Su6kvXLCd6PKpuS87ccTymaW1z1Jq?=
 =?us-ascii?Q?2HzdIoRp29Ok/TAbgilej5mJFDgq+1/d97TO73XNXYO3vZOBxnfNAjvTvUWp?=
 =?us-ascii?Q?KXFqxAd5dZUfiZMG8433bYtTm97nzcYsugfJesHqnUKPPlXCOTyEwZeaH1PD?=
 =?us-ascii?Q?HH3HQQPvAE/ComABasmGIhGL/LoO5u3FkJuELhfM9uQnJmoxeAkeQJVzZ0oE?=
 =?us-ascii?Q?BOGN98/3NCxWfjZExnRIP+DGuVZ6ImmOhEjHTbs/I8Y6NLMrDLb8nCpRxDL5?=
 =?us-ascii?Q?Rj8R6DUPLZmct5CIxTEoNtxqiNrmvshc3pvkNSZ3GFMc6OaWGJW8AtrwlzDu?=
 =?us-ascii?Q?1Njg8zAOHA=3D=3D?=
X-OriginatorOrg: sraoss.co.jp
X-MS-Exchange-CrossTenant-Network-Message-Id: 
 d82a0069-214f-4afb-0375-08deb00b76ce
X-MS-Exchange-CrossTenant-AuthSource: OSZP286MB2160.JPNP286.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 May 2026 09:47:21.9015
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 75db6c32-352e-4063-ae79-cafb8623f6db
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 
 W7TTE980uExx1tUj1XDu0PYXT+6a2n0AGNFUJCDldLCFfpvzMHnfjyb8oT8pqdx5kMQmsOrEPub+q8DsJcDORQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TY1P286MB3278
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: 
 <https://www.postgresql.org/message-id/20260512184721.bf257567ab52d7d48a962350%40sraoss.co.jp>
Precedence: bulk

On Mon, 4 May 2026 15:44:57 -0500
Sami Imseih <samimseih@gmail.com> wrote:

Thank you for your review!

> > > 1/
> > >
> > > +            relid = RangeVarGetRelid(vrel->relation, NoLock, false);
> > >
> > > Should this be called with "true" as the 3rd (missing_ok) argument, otherwise
> > > we end up with an error instead of a "--- relation no longer exists" log. right?
> >
> > No, it should be false. If missing_ok is true, VACUUM (SKIP_LOCKED) on a not-existing
> > table would emit a "skipping vacuum of ... --- relation no longer exists" message, but
> > it should be "relation ... does not exist".
> 
> Yeah you are right.
> 
> But, after looking more into this, I still think the
> expand_vacuum_rel() changes can be
> improved. The branching
> -                */
> -               if (!OidIsValid(relid))
> +               if (!(options & VACOPT_SKIP_LOCKED))
>                 {
> -                       if (options & VACOPT_VACUUM)
> -                               ereport(WARNING,
> -
> (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
> -                                                errmsg("skipping
> vacuum of \"%s\" --- lock not available",
> -
> vrel->relation->relname)));
> -                       else
> -                               ereport(WARNING,
> -
> (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
> -                                                errmsg("skipping
> analyze of \"%s\" --- lock not available",
> +                       relid = RangeVarGetRelidExtended(vrel->relation,
> +
>                   AccessShareLock,
> +
>                   0, NULL, NULL);
> +                       if (!OidIsValid(relid))
> +                               return vacrels;
> +               }
> +               else
> +               {
> +                       /* Get relid for reporting before taking a lock */
> +                       relid = RangeVarGetRelid(vrel->relation, NoLock, false);
> +
> +                       if (!ConditionalLockRelationOid(relid, AccessShareLock))
> 
> is not needed. We can continue just using RangeVarGetRelidExtended()
> with the rvr_opts and an AccessExclusiveLock, and once we need to
> report that we cannot obtain the lock, RangeVarGetRelid() can be
> called at that point for the purpose of calling
> pgstat_report_skipped_vacuum_analyze(). This is safer than calling
> ConditionalLockRelationOid() on a relid obtained with NoLock. See
> attached v6.

It seems good to me.

Initially, I was concerned that something might go wrong if a concurrent
session performed DROP TABLE or ALTER TABLE RENAME between RangeVarGetRelidExtended()
and RangeVarGetRelid(), but I could not find any actual issue. Even when the table
name is changed, the correct statistics entry is updated correctly.

So I'm fine with your version.

Regards,
Yugo Nagata

> >> 2/
> >>
> >> Can the isolation tests
> >> src/test/isolation/specs/vacuum-skip-locked.spec be updated
> >> to check pg_stat_user_tables as well?
> 
> > Yes, we can. I've attached an updated patch including that test.
> 
> > While working on the test, I noticed that skipped FULL VACUUM was counted
> > in the previous patch, so I fixed it not to avoid counting those cases.
> 
> The tests looks good to me.
> 
> > The names of the new fields are still open. The current pattern is
> > "last_skipped_..." and "skipped_..._count". Alternatively, we could use
> > "..._last_skip" and "..._skip_count", which would be consistent with
> > slotsync_skip_count and slosync_last_skip.
> 
> > Which do you think is better?
> 
> I think last_skipped_* is better since we use last_vacuum, last_autovacuum, etc.
> 
> --
> Sami


-- 
Yugo Nagata <nagata@sraoss.co.jp>