Hello!

+ TimestampTz last_skipped_vacuum_time; /* user initiated vacuum */
+ PgStat_Counter skipped_vacuum_count;
+ TimestampTz last_skipped_autovacuum_time; /* autovacuum initiated */
+ PgStat_Counter skipped_autovacuum_count;
+ TimestampTz last_skipped_analyze_time; /* user initiated */
+ PgStat_Counter skipped_analyze_count;
+ TimestampTz last_skipped_autoanalyze_time; /* autovacuum initiated */
+ PgStat_Counter skipped_autoanalyze_count;
+

Doesn't these also require a PGSTAT_FILE_FORMAT_ID change?


There's also an asymmetric case for the skipped counters, is that intentional?

| Command                                 | `skipped_vacuum_count` |
`skipped_analyze_count` |
|-----------------------------------------|------------------------|-------------------------|
| `VACUUM (FULL, ANALYZE, SKIP_LOCKED) t` | 0                      | 1
                      |
| `VACUUM (ANALYZE, SKIP_LOCKED) t`       | 1                      | 1
                      |
| `VACUUM (FULL, SKIP_LOCKED) t`          | 0                      | 0
                      |


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

A DROP TABLE can cause a missed skip in statistics, which is
reproducible with a custom injection point and tap test, see the
attached patch. The race window is quite minimal, but it exists.

Attachment: 0001-DROP-TABLE-race-in-expand_vacuum_rel-skip-lock-path.patch.nocfbot
Description: Binary data

Reply via email to