pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix invalidation of local pgstats references for entry reinitial 4+ messages / 1 participants [nested] [flat]
* pgsql: Fix invalidation of local pgstats references for entry reinitial @ 2024-12-09 01:46 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 4+ messages in thread From: Michael Paquier @ 2024-12-09 01:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix invalidation of local pgstats references for entry reinitialization 818119afccd3 has introduced the "generation" concept in pgstats entries, incremented a counter when a pgstats entry is reinitialized, but it did not count on the fact that backends still holding local references to such entries need to be refreshed if the cache age is outdated. The previous logic only updated local references when an entry was dropped, but it needs also to consider entries that are reinitialized. This matters for replication slot stats (as well as custom pgstats kinds in 18~), where concurrent drops and creates of a slot could cause incorrect stats to be locally referenced. This would lead to an assertion failure at shutdown when writing out the stats file, as the backend holding an outdated local reference would not be able to drop during its shutdown sequence the stats entry that should be dropped, as the last process holding a reference to the stats entry. The checkpointer was then complaining about such an entry late in the shutdown sequence, after the shutdown checkpoint is finished with the control file updated, causing the stats file to not be generated. In non-assert builds, the entry would just be skipped with the stats file written. Note that only logical replication slots use statistics. A test case based on TAP is added to test_decoding, where a persistent connection peeking at a slot's data is kept with concurrent drops and creates of the same slot. This is based on the isolation test case that Anton has sent. As it requires a node shutdown with a check to make sure that the stats file is written with this specific sequence of events, TAP is used instead. Reported-by: Anton A. Melnikov Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/56bf8ff9-dd8c-47b2-872a-748ede82af99@postgrespro.ru Backpatch-through: 15 Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/1df1e1e787621cefeb9741b6e9fe74cd38143eee Modified Files -------------- contrib/test_decoding/t/001_repl_stats.pl | 60 +++++++++++++++++++++++++++++++ src/backend/utils/activity/pgstat_shmem.c | 11 ++++-- 2 files changed, 69 insertions(+), 2 deletions(-) ^ permalink raw reply [nested|flat] 4+ messages in thread
* pgsql: Fix invalidation of local pgstats references for entry reinitial @ 2024-12-09 01:46 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 4+ messages in thread From: Michael Paquier @ 2024-12-09 01:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix invalidation of local pgstats references for entry reinitialization 818119afccd3 has introduced the "generation" concept in pgstats entries, incremented a counter when a pgstats entry is reinitialized, but it did not count on the fact that backends still holding local references to such entries need to be refreshed if the cache age is outdated. The previous logic only updated local references when an entry was dropped, but it needs also to consider entries that are reinitialized. This matters for replication slot stats (as well as custom pgstats kinds in 18~), where concurrent drops and creates of a slot could cause incorrect stats to be locally referenced. This would lead to an assertion failure at shutdown when writing out the stats file, as the backend holding an outdated local reference would not be able to drop during its shutdown sequence the stats entry that should be dropped, as the last process holding a reference to the stats entry. The checkpointer was then complaining about such an entry late in the shutdown sequence, after the shutdown checkpoint is finished with the control file updated, causing the stats file to not be generated. In non-assert builds, the entry would just be skipped with the stats file written. Note that only logical replication slots use statistics. A test case based on TAP is added to test_decoding, where a persistent connection peeking at a slot's data is kept with concurrent drops and creates of the same slot. This is based on the isolation test case that Anton has sent. As it requires a node shutdown with a check to make sure that the stats file is written with this specific sequence of events, TAP is used instead. Reported-by: Anton A. Melnikov Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/56bf8ff9-dd8c-47b2-872a-748ede82af99@postgrespro.ru Backpatch-through: 15 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/da99fedf8c1726326868d37a0e886caf9107d040 Modified Files -------------- contrib/test_decoding/t/001_repl_stats.pl | 60 +++++++++++++++++++++++++++++++ src/backend/utils/activity/pgstat_shmem.c | 11 ++++-- 2 files changed, 69 insertions(+), 2 deletions(-) ^ permalink raw reply [nested|flat] 4+ messages in thread
* pgsql: Fix invalidation of local pgstats references for entry reinitial @ 2024-12-09 01:46 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 4+ messages in thread From: Michael Paquier @ 2024-12-09 01:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix invalidation of local pgstats references for entry reinitialization 818119afccd3 has introduced the "generation" concept in pgstats entries, incremented a counter when a pgstats entry is reinitialized, but it did not count on the fact that backends still holding local references to such entries need to be refreshed if the cache age is outdated. The previous logic only updated local references when an entry was dropped, but it needs also to consider entries that are reinitialized. This matters for replication slot stats (as well as custom pgstats kinds in 18~), where concurrent drops and creates of a slot could cause incorrect stats to be locally referenced. This would lead to an assertion failure at shutdown when writing out the stats file, as the backend holding an outdated local reference would not be able to drop during its shutdown sequence the stats entry that should be dropped, as the last process holding a reference to the stats entry. The checkpointer was then complaining about such an entry late in the shutdown sequence, after the shutdown checkpoint is finished with the control file updated, causing the stats file to not be generated. In non-assert builds, the entry would just be skipped with the stats file written. Note that only logical replication slots use statistics. A test case based on TAP is added to test_decoding, where a persistent connection peeking at a slot's data is kept with concurrent drops and creates of the same slot. This is based on the isolation test case that Anton has sent. As it requires a node shutdown with a check to make sure that the stats file is written with this specific sequence of events, TAP is used instead. Reported-by: Anton A. Melnikov Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/56bf8ff9-dd8c-47b2-872a-748ede82af99@postgrespro.ru Backpatch-through: 15 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/dc5f9054186a0bbee494e0a6f72c2e9e6a15d3f0 Modified Files -------------- contrib/test_decoding/t/001_repl_stats.pl | 60 +++++++++++++++++++++++++++++++ src/backend/utils/activity/pgstat_shmem.c | 11 ++++-- 2 files changed, 69 insertions(+), 2 deletions(-) ^ permalink raw reply [nested|flat] 4+ messages in thread
* pgsql: Fix invalidation of local pgstats references for entry reinitial @ 2024-12-09 01:46 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 4+ messages in thread From: Michael Paquier @ 2024-12-09 01:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix invalidation of local pgstats references for entry reinitialization 818119afccd3 has introduced the "generation" concept in pgstats entries, incremented a counter when a pgstats entry is reinitialized, but it did not count on the fact that backends still holding local references to such entries need to be refreshed if the cache age is outdated. The previous logic only updated local references when an entry was dropped, but it needs also to consider entries that are reinitialized. This matters for replication slot stats (as well as custom pgstats kinds in 18~), where concurrent drops and creates of a slot could cause incorrect stats to be locally referenced. This would lead to an assertion failure at shutdown when writing out the stats file, as the backend holding an outdated local reference would not be able to drop during its shutdown sequence the stats entry that should be dropped, as the last process holding a reference to the stats entry. The checkpointer was then complaining about such an entry late in the shutdown sequence, after the shutdown checkpoint is finished with the control file updated, causing the stats file to not be generated. In non-assert builds, the entry would just be skipped with the stats file written. Note that only logical replication slots use statistics. A test case based on TAP is added to test_decoding, where a persistent connection peeking at a slot's data is kept with concurrent drops and creates of the same slot. This is based on the isolation test case that Anton has sent. As it requires a node shutdown with a check to make sure that the stats file is written with this specific sequence of events, TAP is used instead. Reported-by: Anton A. Melnikov Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/56bf8ff9-dd8c-47b2-872a-748ede82af99@postgrespro.ru Backpatch-through: 15 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/ae77bcc3aa1b869568c6b01726e4f1276f7381bb Modified Files -------------- contrib/test_decoding/t/001_repl_stats.pl | 60 +++++++++++++++++++++++++++++++ src/backend/utils/activity/pgstat_shmem.c | 11 ++++-- 2 files changed, 69 insertions(+), 2 deletions(-) ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2024-12-09 01:46 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-12-09 01:46 pgsql: Fix invalidation of local pgstats references for entry reinitial Michael Paquier <michael@paquier.xyz> 2024-12-09 01:46 pgsql: Fix invalidation of local pgstats references for entry reinitial Michael Paquier <michael@paquier.xyz> 2024-12-09 01:46 pgsql: Fix invalidation of local pgstats references for entry reinitial Michael Paquier <michael@paquier.xyz> 2024-12-09 01:46 pgsql: Fix invalidation of local pgstats references for entry reinitial Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox