Thread: statistics import and export: another difference in dump/restore

Hi Jeff and Corey,
Thanks for fixing the bug related to materialized view statistics. I
have now submitted patches so that the test compares statistics as
well. [1]. However, it is showing a failure on windows only [2].
regress_log has the following difference.

@@ -444546,7 +444546,7 @@
  'relname', 'pagg_tab_para_p1',
  'relpages', '54'::integer,
  'reltuples', '12000'::real,
- 'relallvisible', '0'::integer,
+ 'relallvisible', '54'::integer,
  'relallfrozen', '0'::integer
 );
 SELECT * FROM pg_catalog.pg_restore_attribute_stats(
@@ -444584,7 +444584,7 @@
  'relname', 'pagg_tab_para_p2',
  'relpages', '45'::integer,
  'reltuples', '10000'::real,
- 'relallvisible', '0'::integer,
+ 'relallvisible', '45'::integer,
  'relallfrozen', '0'::integer
 );
 SELECT * FROM pg_catalog.pg_restore_attribute_stats(
@@ -444622,7 +444622,7 @@
  'relname', 'pagg_tab_para_p3',
  'relpages', '36'::integer,
  'reltuples', '8000'::real,
- 'relallvisible', '0'::integer,
+ 'relallvisible', '36'::integer,
  'relallfrozen', '0'::integer
 );
 SELECT * FROM pg_catalog.pg_restore_attribute_stats(
@@ -450930,11 +450930,23 @@
  'version', '180000'::integer,
  'schemaname', 'public',
  'relname', 'test_io_bulkwrite_strategy',
- 'relpages', '0'::integer,
- 'reltuples', '-1'::real,
+ 'relpages', '1'::integer,
+ 'reltuples', '100'::real,
  'relallvisible', '0'::integer,
  'relallfrozen', '0'::integer
 );
+SELECT * FROM pg_catalog.pg_restore_attribute_stats(
+ 'version', '180000'::integer,
+ 'schemaname', 'public',
+ 'relname', 'test_io_bulkwrite_strategy',
+ 'attname', 'i',
+ 'inherited', 'f'::boolean,
+ 'null_frac', '0'::real,
+ 'avg_width', '4'::integer,
+ 'n_distinct', '-1'::real,
+ 'histogram_bounds',

'{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100}'::text,
+ 'correlation', '1'::real
+);

First 3 and 5th differences are new differences but we have seen
something similar to the 4th one previously.

I am not able to reproduce the failure even after running it 30 times
on my linux laptop.

[1] https://www.postgresql.org/message-id/CAExHW5vVFtCejh+UYzNxMGSXOfJ_1xwi5aQHQfemqJgFmkyK5Q@mail.gmail.com
[2] https://cirrus-ci.com/task/5164175841820672

--
Best Wishes,
Ashutosh Bapat

Hi,


On Tue, Apr 1, 2025 at 12:54 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> [1] https://www.postgresql.org/message-id/CAExHW5vVFtCejh+UYzNxMGSXOfJ_1xwi5aQHQfemqJgFmkyK5Q@mail.gmail.com
> [2] https://cirrus-ci.com/task/5164175841820672

I have added this to PG 18 open items. It might be too early to call
this an open item but 1. We have already started tracking open items
2. There's follow on work.

Once we fix this issue, we need to enable statistics dump and
comparison in pg_upgrade/002_pg_upgrade using the attached patch.

--
Best Wishes,
Ashutosh Bapat

On Wed, 2025-04-02 at 15:35 +0530, Ashutosh Bapat wrote:
> Once we fix this issue, we need to enable statistics dump and
> comparison in pg_upgrade/002_pg_upgrade using the attached patch.

The diff appears to be an issue in 002_pg_upgrade.pl introduced in
172259afb5. There are two dumps taken from $oldnode: $src_dump is taken
on line 321, and then $dump_file is taken on line 339. $dump_file is
used for the restore, but then $src_dump is used for the comparison.

If autovacuum were off, then $src_dump and $dump_file should be the
same, but autovacuum is not disabled until line 415. Furthermore,
there's an initdb between lines 321 and 339, allowing autovacuum enough
time to activate. Otherwise we probably wouldn't have noticed.

You could disable autovacuum earlier; though not too early, because
allowing it time to run improves test coverage for stats. Or, you could
use the same $src_dump for both restoration and comparison, but it
looks like you wanted coverage of the --create option. (Aside: why
parallel restore there? Is that just for test coverage or was there a
performance reason?)

Regards,
    Jeff Davis

On Wed, Apr 2, 2025 at 10:55 PM Jeff Davis <pgsql@j-davis.com> wrote:
>
> On Wed, 2025-04-02 at 15:35 +0530, Ashutosh Bapat wrote:
> > Once we fix this issue, we need to enable statistics dump and
> > comparison in pg_upgrade/002_pg_upgrade using the attached patch.
>
> The diff appears to be an issue in 002_pg_upgrade.pl introduced in
> 172259afb5. There are two dumps taken from $oldnode: $src_dump is taken
> on line 321, and then $dump_file is taken on line 339. $dump_file is
> used for the restore, but then $src_dump is used for the comparison.
>
> If autovacuum were off, then $src_dump and $dump_file should be the
> same, but autovacuum is not disabled until line 415. Furthermore,
> there's an initdb between lines 321 and 339, allowing autovacuum enough
> time to activate. Otherwise we probably wouldn't have noticed.

Thanks for the analysis. This is useful. I will post a fix on the
original thread. I have assigned myself and fixed the open item entry.

>
> You could disable autovacuum earlier; though not too early, because
> allowing it time to run improves test coverage for stats. Or, you could
> use the same $src_dump for both restoration and comparison, but it
> looks like you wanted coverage of the --create option. (Aside: why
> parallel restore there? Is that just for test coverage or was there a
> performance reason?)

For performance.

--
Best Wishes,
Ashutosh Bapat