Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6059946

Browse files
committed
Fix assertion failure when updating stats_fetch_consistency in a transaction
An update of the GUC stats_fetch_consistency in a transaction would beable to trigger an assertion when doing cache->snapshot. In this case,when retrieving a pgstat entry after the switch, a new snapshot would berebuilt, confusing pgstat_build_snapshot() because a snapshot is alreadycached with an unexpected mode ("cache").In order to fix this problem, this commit adds a flag to force asnapshot clear each time this GUC is changed. Some tests are added tocheck, while on it.Some optimizations in avoiding the snapshot clear should be possibledepending on what is cached and the current GUC value, I guess, but thissolution is simple, and ensures that the state of the cache is updatedeach time a new pgstat entry is fetched, hence being consistent with thelevel wanted by the client that has set the GUC.Note that cache->none and snapshot->none would not cause issues, asfetching a pgstat entry would be retrieved from shared memory on thesecond attempt, however a snapshot would still be cached. Similarly,none->snapshot and none->cache would build a new snapshot on the secondfetch attempt. Finally, snapshot->cache would cache a new snapshot onthe second attempt.Reported-by: Alexander LakhinAuthor: Kyotaro HoriguchiDiscussion:https://postgr.es/m/17804-2a118cd046f2d0e5@postgresql.orgbackpatch-through: 15
1 parent4d47eff commit6059946

File tree

6 files changed

+118
-5
lines changed

6 files changed

+118
-5
lines changed

‎doc/src/sgml/config.sgml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8219,8 +8219,9 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
82198219
called. When set to <literal>snapshot</literal>, the first statistics
82208220
access caches all statistics accessible in the current database, until
82218221
the end of the transaction unless
8222-
<function>pg_stat_clear_snapshot()</function> is called. The default
8223-
is <literal>cache</literal>.
8222+
<function>pg_stat_clear_snapshot()</function> is called. Changing this
8223+
parameter in a transaction discards the statistics snapshot.
8224+
The default is <literal>cache</literal>.
82248225
</para>
82258226
<note>
82268227
<para>

‎src/backend/utils/activity/pgstat.c

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@
103103
#include"storage/lwlock.h"
104104
#include"storage/pg_shmem.h"
105105
#include"storage/shmem.h"
106-
#include"utils/guc.h"
106+
#include"utils/guc_hooks.h"
107107
#include"utils/memutils.h"
108108
#include"utils/pgstat_internal.h"
109109
#include"utils/timestamp.h"
@@ -227,6 +227,13 @@ static dlist_head pgStatPending = DLIST_STATIC_INIT(pgStatPending);
227227
*/
228228
staticboolpgStatForceNextFlush= false;
229229

230+
/*
231+
* Force-clear existing snapshot before next use when stats_fetch_consistency
232+
* is changed.
233+
*/
234+
staticboolforce_stats_snapshot_clear= false;
235+
236+
230237
/*
231238
* For assertions that check pgstat is not used before initialization / after
232239
* shutdown.
@@ -760,7 +767,8 @@ pgstat_reset_of_kind(PgStat_Kind kind)
760767
* request will cause new snapshots to be read.
761768
*
762769
* This is also invoked during transaction commit or abort to discard
763-
* the no-longer-wanted snapshot.
770+
* the no-longer-wanted snapshot. Updates of stats_fetch_consistency can
771+
* cause this routine to be called.
764772
*/
765773
void
766774
pgstat_clear_snapshot(void)
@@ -787,6 +795,9 @@ pgstat_clear_snapshot(void)
787795
* forward the reset request.
788796
*/
789797
pgstat_clear_backend_activity_snapshot();
798+
799+
/* Reset this flag, as it may be possible that a cleanup was forced. */
800+
force_stats_snapshot_clear= false;
790801
}
791802

792803
void*
@@ -885,6 +896,9 @@ pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, Oid objoid)
885896
TimestampTz
886897
pgstat_get_stat_snapshot_timestamp(bool*have_snapshot)
887898
{
899+
if (force_stats_snapshot_clear)
900+
pgstat_clear_snapshot();
901+
888902
if (pgStatLocal.snapshot.mode==PGSTAT_FETCH_CONSISTENCY_SNAPSHOT)
889903
{
890904
*have_snapshot= true;
@@ -929,6 +943,9 @@ pgstat_snapshot_fixed(PgStat_Kind kind)
929943
staticvoid
930944
pgstat_prep_snapshot(void)
931945
{
946+
if (force_stats_snapshot_clear)
947+
pgstat_clear_snapshot();
948+
932949
if (pgstat_fetch_consistency==PGSTAT_FETCH_CONSISTENCY_NONE||
933950
pgStatLocal.snapshot.stats!=NULL)
934951
return;
@@ -1671,3 +1688,18 @@ pgstat_reset_after_failure(void)
16711688
/* and drop variable-numbered ones */
16721689
pgstat_drop_all_entries();
16731690
}
1691+
1692+
/*
1693+
* GUC assign_hook for stats_fetch_consistency.
1694+
*/
1695+
void
1696+
assign_stats_fetch_consistency(intnewval,void*extra)
1697+
{
1698+
/*
1699+
* Changing this value in a transaction may cause snapshot state
1700+
* inconsistencies, so force a clear of the current snapshot on the next
1701+
* snapshot build attempt.
1702+
*/
1703+
if (pgstat_fetch_consistency!=newval)
1704+
force_stats_snapshot_clear= true;
1705+
}

‎src/backend/utils/misc/guc_tables.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4821,7 +4821,7 @@ struct config_enum ConfigureNamesEnum[] =
48214821
},
48224822
&pgstat_fetch_consistency,
48234823
PGSTAT_FETCH_CONSISTENCY_CACHE,stats_fetch_consistency,
4824-
NULL,NULL,NULL
4824+
NULL,assign_stats_fetch_consistency,NULL
48254825
},
48264826

48274827
{

‎src/include/utils/guc_hooks.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ extern void assign_search_path(const char *newval, void *extra);
121121
externboolcheck_session_authorization(char**newval,void**extra,GucSourcesource);
122122
externvoidassign_session_authorization(constchar*newval,void*extra);
123123
externvoidassign_session_replication_role(intnewval,void*extra);
124+
externvoidassign_stats_fetch_consistency(intnewval,void*extra);
124125
externboolcheck_ssl(bool*newval,void**extra,GucSourcesource);
125126
externboolcheck_stage_log_stats(bool*newval,void**extra,GucSourcesource);
126127
externboolcheck_synchronous_standby_names(char**newval,void**extra,

‎src/test/regress/expected/stats.out

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,65 @@ SELECT pg_stat_get_snapshot_timestamp();
985985

986986
COMMIT;
987987
----
988+
-- Changing stats_fetch_consistency in a transaction.
989+
----
990+
BEGIN;
991+
-- Stats filled under the cache mode
992+
SET LOCAL stats_fetch_consistency = cache;
993+
SELECT pg_stat_get_function_calls(0);
994+
pg_stat_get_function_calls
995+
----------------------------
996+
997+
(1 row)
998+
999+
SELECT pg_stat_get_snapshot_timestamp() IS NOT NULL AS snapshot_ok;
1000+
snapshot_ok
1001+
-------------
1002+
f
1003+
(1 row)
1004+
1005+
-- Success in accessing pre-existing snapshot data.
1006+
SET LOCAL stats_fetch_consistency = snapshot;
1007+
SELECT pg_stat_get_snapshot_timestamp() IS NOT NULL AS snapshot_ok;
1008+
snapshot_ok
1009+
-------------
1010+
f
1011+
(1 row)
1012+
1013+
SELECT pg_stat_get_function_calls(0);
1014+
pg_stat_get_function_calls
1015+
----------------------------
1016+
1017+
(1 row)
1018+
1019+
SELECT pg_stat_get_snapshot_timestamp() IS NOT NULL AS snapshot_ok;
1020+
snapshot_ok
1021+
-------------
1022+
t
1023+
(1 row)
1024+
1025+
-- Snapshot cleared.
1026+
SET LOCAL stats_fetch_consistency = none;
1027+
SELECT pg_stat_get_snapshot_timestamp() IS NOT NULL AS snapshot_ok;
1028+
snapshot_ok
1029+
-------------
1030+
f
1031+
(1 row)
1032+
1033+
SELECT pg_stat_get_function_calls(0);
1034+
pg_stat_get_function_calls
1035+
----------------------------
1036+
1037+
(1 row)
1038+
1039+
SELECT pg_stat_get_snapshot_timestamp() IS NOT NULL AS snapshot_ok;
1040+
snapshot_ok
1041+
-------------
1042+
f
1043+
(1 row)
1044+
1045+
ROLLBACK;
1046+
----
9881047
-- pg_stat_have_stats behavior
9891048
----
9901049
-- fixed-numbered stats exist

‎src/test/regress/sql/stats.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,26 @@ SELECT pg_stat_clear_snapshot();
476476
SELECT pg_stat_get_snapshot_timestamp();
477477
COMMIT;
478478

479+
----
480+
-- Changing stats_fetch_consistency in a transaction.
481+
----
482+
BEGIN;
483+
-- Stats filled under the cache mode
484+
SET LOCAL stats_fetch_consistency= cache;
485+
SELECT pg_stat_get_function_calls(0);
486+
SELECT pg_stat_get_snapshot_timestamp()IS NOT NULLAS snapshot_ok;
487+
-- Success in accessing pre-existing snapshot data.
488+
SET LOCAL stats_fetch_consistency= snapshot;
489+
SELECT pg_stat_get_snapshot_timestamp()IS NOT NULLAS snapshot_ok;
490+
SELECT pg_stat_get_function_calls(0);
491+
SELECT pg_stat_get_snapshot_timestamp()IS NOT NULLAS snapshot_ok;
492+
-- Snapshot cleared.
493+
SET LOCAL stats_fetch_consistency= none;
494+
SELECT pg_stat_get_snapshot_timestamp()IS NOT NULLAS snapshot_ok;
495+
SELECT pg_stat_get_function_calls(0);
496+
SELECT pg_stat_get_snapshot_timestamp()IS NOT NULLAS snapshot_ok;
497+
ROLLBACK;
498+
479499
----
480500
-- pg_stat_have_stats behavior
481501
----

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp