[YSQL] Manual ANALYZE reset of auto analyze counters races with mutation reporting

Jira Link: DB-23656

Description

Correction to the original filing. This issue first claimed that auto analyze does not learn about a manual ANALYZE. That was wrong. b6bbc7a663df2252865be498931bd4484ac7c224 (#30578) added exactly that, three months before the failures that prompted this. The real problem is narrower: the reset clears the persisted counters, but mutations still buffered on the tserver are added after it, so the trigger can re-arm immediately and the redundant ANALYZE happens anyway.

What exists today

A full manual ANALYZE on a YB relation clears the persisted counters, in commands/analyze.c:

/*
 * YB: yb auto-analyze treats a full manual ANALYZE as making table
 * statistics fresh enough that it does not need to immediately re-analyze
 * based on mutations collected before this ANALYZE. Clear only the persisted
 * YCQL mutation stats; do not coordinate with auto-analyze service-local
 * buffers or caches.
 */
if (!inh && IsYBRelation(onerel) && va_cols == NIL && ...)
    YBCResetAutoAnalyzeMutationCounters(YBCGetDatabaseOid(onerel), YbGetRelfileNodeId(onerel));

That reaches ResetPgAutoAnalyzeMutationCounts, which assigns the mutations column of the pg_auto_analyze_table stateful service row for that table, through NewSetMutationsOp.

The gap

The comment above names it: the reset does not coordinate with service local buffers. Row level mutations accumulate in the tserver's PgMutationCounter and are flushed to that row periodically, every ysql_node_level_mutation_reporting_interval_ms, five seconds by default. Mutations that happened before the ANALYZE but had not yet been flushed are therefore added after the reset assigned the column, and the trigger can cross its threshold again straight away.

The observed sequence in #34021 is consistent with this, though I have not instrumented it:

• The suite inserts about 5048 rows into a table, then immediately runs ANALYZE on it.
• The reset assigns the persisted counter, which at that moment may not yet include those rows.
• The sender flushes them afterwards. The threshold for that table is 50 + 0.1 * 5048, about 555, so 5048 mutations cross it comfortably.
• Auto analyze runs about 13 seconds after the tserver starts, well after the manual ANALYZE, which is what moved a plan mid run.

Why it is worth closing the gap

• A redundant ANALYZE on a large table is a full sampling pass for no new information.
• ANALYZE is treated as DDL in YSQL and increments the catalog version, to force a catalog cache refresh for the new statistics, so a redundant one also makes every session in that database refresh its catalog cache and contends with user DDL. That cost is visible in #28391 and #28393, and is why ysql_yb_user_ddls_preempt_auto_analyze exists.
• It is a source of test flakiness, as in #34021.

Possible direction

Best effort is enough, since a manual and an automatic ANALYZE can still race and losing that race costs one redundant analyze, which is today's behaviour. Options worth considering include having the reset also discard node local counts accumulated up to that point, or recording the reset with a timestamp or sequence number so that counts collected before it are not applied after it.

Relationship to other issues

#34049 covers the events that should trigger an analyze but do not. Ordering matters between the two. The redundant run this issue removes is currently the only thing that gives a newly created expression index its statistics on an otherwise idle table, so closing this gap first would quietly make that one worse.

Upstream PostgreSQL does not have this particular race. pgstat_report_analyze zeroes changes_since_analyze in the same statistics path that accumulates it, when a full ANALYZE has been run.

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

• I confirm this issue does not contain any sensitive information.