Thread

Re: Track skipped tables during autovacuum and autoanalyze

Sami Imseih <samimseih@gmail.com> — 2026-05-04T20:44:57Z

Thanks for the update!

> >
> > 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.

>> 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