Thread: Stats sender and 2pc minor problem

Hello.

Statistics sender logic during usual commit and two-phase commit do not strictly matches each other and that leads to
delta_live_tuples added to n_live_tup in case of truncate in two phase commit.

That can be see in following example:

CREATE TABLE trunc_stats_test5(id serial);
INSERT INTO trunc_stats_test5 DEFAULT VALUES;
INSERT INTO trunc_stats_test5 DEFAULT VALUES;
INSERT INTO trunc_stats_test5 DEFAULT VALUES;
BEGIN;
TRUNCATE trunc_stats_test5;
PREPARE TRANSACTION 'twophase_stats';
COMMIT PREPARED 'twophase_stats';

After that pg_stat_user_tables will have n_live_tup = 3 instead of 0.

Fix along with test is attached.


--
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

Stas Kelvich <s.kelvich@postgrespro.ru> writes:
> Statistics sender logic during usual commit and two-phase commit do not
> strictly matches each other and that leads to delta_live_tuples added to
> n_live_tup in case of truncate in two phase commit.

Yeah, that code says it's supposed to match AtEOXact_PgStat, but it
doesn't.

I pushed this, but without the regression test case, which would have
failed outright in any test run with max_prepared_transactions = 0.
Timing sensitivity is another problem.  In the commit that created this
discrepancy, d42358efb, Alvaro had tried to add regression coverage for
this area, but we ended up backing it out because it failed too often
in the buildfarm.

TBH, now that I look at it, I think that d42358efb was fundamentally
wrong and this patch is just continuing down the same wrong path.
Having the stats collector respond to a TRUNCATE like this cannot
work reliably, because the "it got truncated" flag will arrive at
the stats collector asynchronously, perhaps quite some time later
than the truncate occurred.  When that happens, we may throw away
live/dead tuple count updates from transactions that actually happened
after the truncate but chanced to report first.

I wonder if we could make that better by making the stats collector
track stats by relfilenode rather than table OID.  It'd be a pretty
major logic change, though, to serve a corner case.
        regards, tom lane

Tom Lane wrote:
> Stas Kelvich <s.kelvich@postgrespro.ru> writes:
> > Statistics sender logic during usual commit and two-phase commit do not
> > strictly matches each other and that leads to delta_live_tuples added to
> > n_live_tup in case of truncate in two phase commit.
> 
> Yeah, that code says it's supposed to match AtEOXact_PgStat, but it
> doesn't.

Hmm, oops.

> I pushed this, but without the regression test case, which would have
> failed outright in any test run with max_prepared_transactions = 0.

I agree that that was the right approach.  Thanks for taking care of it!

> I wonder if we could make that better by making the stats collector
> track stats by relfilenode rather than table OID.  It'd be a pretty
> major logic change, though, to serve a corner case.

Hm, that's an idea.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services