Re: pgstat: Flush some statistics within running transactions, take 2

Hi,

On Fri, Aug 21, 2026 at 10:57:13AM -0500, Sami Imseih wrote:
> Hi,
>
> I posted v7

Thanks!

I focused on 0002 for now, as the second comment below could also impact 0001.

=== 1

> Done in v7. pgstat_report_stat() now uses PG_TRY/PG_FINALLY to clear
> pgStatFlushInProgress on all exit paths, and the reset at entry became
> an assert.

That makes sense but I think we still have a database/relation or index ordering
issue.

If pgstat_prep_database_pending() errors, the shared index (or relation) stats
have already been updated, while the database stats and flushed baseline have not.

I wonder if pgstat_prep_database_pending() should be called before the first
shared stat update, or if the callback should ensure that an error cannot leave
the flush partially applied?

=== 2

> Done in v7. The relation flush path now compares only the
> non-transactional group before taking the lock for an in-transaction
> flush. If there are no changes in the non-transactional stats, it
> returns PGSTAT_FLUSH_PARTIAL immediately.

+ * counts and flushed are zeroed on allocation and no field write ever
+ * touches the padding, so these byte compares are safe

I'm not sure this is guaranteed by C11. Section 6.2.6.1 paragraph 6 (see [1])
says:

"
When a value is stored in an object of structure or union type, including in a
member object, the bytes of the object representation that correspond to any
padding bytes take unspecified values.
"

Moreover, the patch does a structure assignment:

+ if (!xact_boundary)
+ {
+ lstats->tab.flushed = lstats->tab.counts;

And the corresponding footnote explicitly says:

"
Thus, for example, structure assignment need not copy any padding bits
"

So, as PgStat_TableCountsTxn contains padding:

(gdb) ptype /o struct PgStat_TableCountsTxn
/* offset | size */ type = struct PgStat_TableCountsTxn {
/* 0 | 8 */ PgStat_Counter tuples_inserted;
/* 8 | 8 */ PgStat_Counter tuples_updated;
/* 16 | 8 */ PgStat_Counter tuples_deleted;
/* 24 | 8 */ PgStat_Counter tuples_hot_updated;
/* 32 | 8 */ PgStat_Counter tuples_newpage_updated;
/* 40 | 1 */ _Bool truncdropped;
/* XXX 7-byte hole */
/* 48 | 8 */ PgStat_Counter delta_live_tuples;
/* 56 | 8 */ PgStat_Counter delta_dead_tuples;
/* 64 | 8 */ PgStat_Counter changed_tuples;

/* total size (bytes): 72 */
}

then, I wonder if using memcmp() here is safe:

+ if (memcmp(&lstats->tab.counts.txn, &lstats->tab.flushed.txn,
+ sizeof(PgStat_TableCountsTxn)) == 0)
+ return PGSTAT_FLUSH_DONE;

as it could report a difference because of the padding even when all their
members are equal. That said that would cause extra work, not missed statistics.

[1]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com