Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Alex Shulgin wrote:
>
>> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> >>
>> >> Another idea would be exposing pgstat_report_stat(true) at SQL level.
>> >> That would eleminate the need for explicit pg_sleep(>=0.5), but we'll
>> >> still need the wait_for_... call to make sure the collector has picked
>> >> it up.
>> >
>> > We already have a stats test that sleeps. Why not add this stuff there,
>> > to avoid making another test slow?
>>
>> Well, if we could come up with a set of statements to test that would
>> produce the end result unambigously, so that we can be certain the stats
>> we check at the end cannot be a result of neat interaction of buggy
>> behavior...
>
> It is always possible that things work just right because two bugs
> cancel each other.
>
>> I'm not sure this is at all possible, but I know for sure it will make
>> debugging the possible fails harder.
>
> Surely if some other patch introduces a failure, nobody will try to
> debug it by looking only at the failures of this test.
>
>> Even with the current approach of checking the stats after every
>> isolated case it's sometimes takes quite a little more than a glance
>> to verify correctness due to side-effects of rollback (ins/upd/del
>> counters are still updated), and the way stats are affecting the dead
>> tuples counter.
>
> Honestly I think pg_regress is not the right tool to test stat counter
> updates. It kind-of works today, but only because we don't stress it
> too much. If you want to create a new test framework for pgstats, and
> include some tests for truncate, be my guest.
Yes, these are all good points. Actually, moving the test to stats.sql
helped me realize the current patch behavior is not strictly correct:
there's a corner case when you insert/update before truncate in
transaction, which is later aborted. I need to take a closer look.
--
Alex
