diff options
| author | Andres Freund <andres@anarazel.de> | 2022-04-16 12:13:31 -0700 |
|---|---|---|
| committer | Andres Freund <andres@anarazel.de> | 2022-04-16 14:44:58 -0700 |
| commit | 4a736a161c306fcfed970e6b649f2f03f465ac24 (patch) | |
| tree | d0b93ecb10a1ef84160726e13a150282bbb70bf0 /src | |
| parent | 9f4f0a0dad4c7422a97d94e4051c08ec6d181dd6 (diff) | |
| download | postgresql-4a736a161c306fcfed970e6b649f2f03f465ac24.tar.gz postgresql-4a736a161c306fcfed970e6b649f2f03f465ac24.zip | |
pgstat: Use correct lock level in pgstat_drop_all_entries().
Previously we didn't, which lead to an assertion failure when resetting
partially loaded statistics. This was encountered on the buildfarm, for
as-of-yet unknown reasons.
Ttighten up a validity check when reading the stats file, verifying 'E'
signals the end of the file (rather than just stopping reading). That's then
used in a test appending to the stats file that crashed before the fix in
pgstat_drop_all_entries().
Reported by buildfarm animals mylodon and kestrel, via Tom Lane.
Discussion: https://postgr.es/m/1656446.1650043715@sss.pgh.pa.us
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/utils/activity/pgstat.c | 4 | ||||
| -rw-r--r-- | src/backend/utils/activity/pgstat_shmem.c | 2 | ||||
| -rw-r--r-- | src/test/recovery/t/029_stats_restart.pl | 30 |
3 files changed, 33 insertions, 3 deletions
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index f658f8f1989..3c3fd0e9b7f 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -1583,6 +1583,10 @@ pgstat_read_statsfile(void) break; } case 'E': + /* check that 'E' actually signals end of file */ + if (fgetc(fpin) != EOF) + goto error; + goto done; default: diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 68ff6f51fcf..89060ef29a0 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -878,7 +878,7 @@ pgstat_drop_all_entries(void) PgStatShared_HashEntry *ps; uint64 not_freed_count = 0; - dshash_seq_init(&hstat, pgStatLocal.shared_hash, false); + dshash_seq_init(&hstat, pgStatLocal.shared_hash, true); while ((ps = dshash_seq_next(&hstat)) != NULL) { if (ps->dropped) diff --git a/src/test/recovery/t/029_stats_restart.pl b/src/test/recovery/t/029_stats_restart.pl index e0478f8af8d..2fe8db88079 100644 --- a/src/test/recovery/t/029_stats_restart.pl +++ b/src/test/recovery/t/029_stats_restart.pl @@ -113,7 +113,23 @@ overwrite_file($og_stats, "ZZZZZZZZZZZZZ"); $node->start; # no stats present due to invalid stats file -$sect = "invalid"; +$sect = "invalid_overwrite"; +is(have_stats('database', $dboid, 0), 'f', "$sect: db stats do not exist"); +is(have_stats('function', $dboid, $funcoid), + 'f', "$sect: function stats do not exist"); +is(have_stats('relation', $dboid, $tableoid), + 'f', "$sect: relation stats do not exist"); + + +## check invalid stats file starting with valid contents, but followed by +## invalid content is handled. + +trigger_funcrel_stat(); +$node->stop; +append_file($og_stats, "XYZ"); +$node->start; + +$sect = "invalid_append"; is(have_stats('database', $dboid, 0), 'f', "$sect: db stats do not exist"); is(have_stats('function', $dboid, $funcoid), 'f', "$sect: function stats do not exist"); @@ -285,7 +301,17 @@ sub overwrite_file { my ($filename, $str) = @_; open my $fh, ">", $filename - or die "could not write \"$filename\": $!"; + or die "could not overwrite \"$filename\": $!"; + print $fh $str; + close $fh; + return; +} + +sub append_file +{ + my ($filename, $str) = @_; + open my $fh, ">>", $filename + or die "could not append to \"$filename\": $!"; print $fh $str; close $fh; return; |