Skip to content

Commit 77947c5

Browse files
tglsfdctglsfdc
committed
Fix up pgstats counting of live and dead tuples to recognize that committed
and aborted transactions have different effects; also teach it not to assume that prepared transactions are always committed. Along the way, simplify the pgstats API by tying counting directly to Relations; I cannot detect any redeeming social value in having stats pointers in HeapScanDesc and IndexScanDesc structures. And fix a few corner cases in which counts might be missed because the relation's pgstat_info pointer hadn't been set.
1 parent cadb783 commit 77947c5

File tree

20 files changed

+802
-414
lines changed

20 files changed

+802
-414
lines changed

‎src/backend/access/gin/ginscan.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gin/ginscan.c,v 1.9 2007/01/31 15:09:45 teodor Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gin/ginscan.c,v 1.10 2007/05/27 03:50:38 tgl Exp $
1212
*-------------------------------------------------------------------------
1313
*/
1414

@@ -189,7 +189,7 @@ newScanKey(IndexScanDesc scan)
189189
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
190190
errmsg("GIN index does not support search with void query")));
191191

192-
pgstat_count_index_scan(&scan->xs_pgstat_info);
192+
pgstat_count_index_scan(scan->indexRelation);
193193
}
194194

195195
Datum

‎src/backend/access/gist/gistget.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.65 2007/04/06 22:33:41 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.66 2007/05/27 03:50:38 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -165,7 +165,7 @@ gistnext(IndexScanDesc scan, ScanDirection dir, ItemPointer tids,
165165
stk->next = NULL;
166166
stk->block = GIST_ROOT_BLKNO;
167167

168-
pgstat_count_index_scan(&scan->xs_pgstat_info);
168+
pgstat_count_index_scan(scan->indexRelation);
169169
}
170170
else if (so->curbuf == InvalidBuffer)
171171
{

‎src/backend/access/hash/hashsearch.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/hash/hashsearch.c,v 1.49 2007/05/03 16:45:58 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/hash/hashsearch.c,v 1.50 2007/05/27 03:50:38 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -127,7 +127,7 @@ _hash_first(IndexScanDesc scan, ScanDirection dir)
127127
ItemPointer current;
128128
OffsetNumber offnum;
129129

130-
pgstat_count_index_scan(&scan->xs_pgstat_info);
130+
pgstat_count_index_scan(rel);
131131

132132
current = &(so->hashso_curpos);
133133
ItemPointerSetInvalid(current);

‎src/backend/access/heap/heapam.c

+22-18
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.232 2007/04/08 01:26:27 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.233 2007/05/27 03:50:38 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -100,7 +100,7 @@ initscan(HeapScanDesc scan, ScanKey key)
100100
if (key != NULL)
101101
memcpy(scan->rs_key, key, scan->rs_nkeys * sizeof(ScanKeyData));
102102

103-
pgstat_count_heap_scan(&scan->rs_pgstat_info);
103+
pgstat_count_heap_scan(scan->rs_rd);
104104
}
105105

106106
/*
@@ -701,6 +701,8 @@ relation_open(Oid relationId, LOCKMODE lockmode)
701701
if (!RelationIsValid(r))
702702
elog(ERROR, "could not open relation with OID %u", relationId);
703703

704+
pgstat_initstats(r);
705+
704706
return r;
705707
}
706708

@@ -743,6 +745,8 @@ try_relation_open(Oid relationId, LOCKMODE lockmode)
743745
if (!RelationIsValid(r))
744746
elog(ERROR, "could not open relation with OID %u", relationId);
745747

748+
pgstat_initstats(r);
749+
746750
return r;
747751
}
748752

@@ -787,6 +791,8 @@ relation_open_nowait(Oid relationId, LOCKMODE lockmode)
787791
if (!RelationIsValid(r))
788792
elog(ERROR, "could not open relation with OID %u", relationId);
789793

794+
pgstat_initstats(r);
795+
790796
return r;
791797
}
792798

@@ -873,8 +879,6 @@ heap_open(Oid relationId, LOCKMODE lockmode)
873879
errmsg("\"%s\" is a composite type",
874880
RelationGetRelationName(r))));
875881

876-
pgstat_initstats(&r->pgstat_info, r);
877-
878882
return r;
879883
}
880884

@@ -903,8 +907,6 @@ heap_openrv(const RangeVar *relation, LOCKMODE lockmode)
903907
errmsg("\"%s\" is a composite type",
904908
RelationGetRelationName(r))));
905909

906-
pgstat_initstats(&r->pgstat_info, r);
907-
908910
return r;
909911
}
910912

@@ -954,8 +956,6 @@ heap_beginscan(Relation relation, Snapshot snapshot,
954956
else
955957
scan->rs_key = NULL;
956958

957-
pgstat_initstats(&scan->rs_pgstat_info, relation);
958-
959959
initscan(scan, key);
960960

961961
return scan;
@@ -1059,7 +1059,7 @@ heap_getnext(HeapScanDesc scan, ScanDirection direction)
10591059
*/
10601060
HEAPDEBUG_3; /* heap_getnext returning tuple */
10611061

1062-
pgstat_count_heap_getnext(&scan->rs_pgstat_info);
1062+
pgstat_count_heap_getnext(scan->rs_rd);
10631063

10641064
return &(scan->rs_ctup);
10651065
}
@@ -1086,6 +1086,10 @@ heap_getnext(HeapScanDesc scan, ScanDirection direction)
10861086
* and return it in *userbuf (so the caller must eventually unpin it); when
10871087
* keep_buf = false, the pin is released and *userbuf is set to InvalidBuffer.
10881088
*
1089+
* stats_relation is the relation to charge the heap_fetch operation against
1090+
* for statistical purposes. (This could be the heap rel itself, an
1091+
* associated index, or NULL to not count the fetch at all.)
1092+
*
10891093
* It is somewhat inconsistent that we ereport() on invalid block number but
10901094
* return false on invalid item number. There are a couple of reasons though.
10911095
* One is that the caller can relatively easily check the block number for
@@ -1101,12 +1105,12 @@ heap_fetch(Relation relation,
11011105
HeapTuple tuple,
11021106
Buffer *userbuf,
11031107
bool keep_buf,
1104-
PgStat_Info *pgstat_info)
1108+
Relation stats_relation)
11051109
{
11061110
/* Assume *userbuf is undefined on entry */
11071111
*userbuf = InvalidBuffer;
11081112
return heap_release_fetch(relation, snapshot, tuple,
1109-
userbuf, keep_buf, pgstat_info);
1113+
userbuf, keep_buf, stats_relation);
11101114
}
11111115

11121116
/*
@@ -1125,7 +1129,7 @@ heap_release_fetch(Relation relation,
11251129
HeapTuple tuple,
11261130
Buffer *userbuf,
11271131
bool keep_buf,
1128-
PgStat_Info *pgstat_info)
1132+
Relation stats_relation)
11291133
{
11301134
ItemPointer tid = &(tuple->t_self);
11311135
ItemId lp;
@@ -1210,9 +1214,9 @@ heap_release_fetch(Relation relation,
12101214
*/
12111215
*userbuf = buffer;
12121216

1213-
/* Count the successful fetch in *pgstat_info, if given. */
1214-
if (pgstat_info != NULL)
1215-
pgstat_count_heap_fetch(pgstat_info);
1217+
/* Count the successful fetch against appropriate rel, if any */
1218+
if (stats_relation != NULL)
1219+
pgstat_count_heap_fetch(stats_relation);
12161220

12171221
return true;
12181222
}
@@ -1517,7 +1521,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
15171521
*/
15181522
CacheInvalidateHeapTuple(relation, heaptup);
15191523

1520-
pgstat_count_heap_insert(&relation->pgstat_info);
1524+
pgstat_count_heap_insert(relation);
15211525

15221526
/*
15231527
* If heaptup is a private copy, release it. Don't forget to copy t_self
@@ -1807,7 +1811,7 @@ heap_delete(Relation relation, ItemPointer tid,
18071811
if (have_tuple_lock)
18081812
UnlockTuple(relation, &(tp.t_self), ExclusiveLock);
18091813

1810-
pgstat_count_heap_delete(&relation->pgstat_info);
1814+
pgstat_count_heap_delete(relation);
18111815

18121816
return HeapTupleMayBeUpdated;
18131817
}
@@ -2269,7 +2273,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
22692273
if (have_tuple_lock)
22702274
UnlockTuple(relation, &(oldtup.t_self), ExclusiveLock);
22712275

2272-
pgstat_count_heap_update(&relation->pgstat_info);
2276+
pgstat_count_heap_update(relation);
22732277

22742278
/*
22752279
* If heaptup is a private copy, release it. Don't forget to copy t_self

‎src/backend/access/index/genam.c

+1-3
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.61 2007/01/20 18:43:35 neilc Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.62 2007/05/27 03:50:38 tgl Exp $
1212
*
1313
* NOTES
1414
* many of the old access method routines have been turned into
@@ -96,8 +96,6 @@ RelationGetIndexScan(Relation indexRelation,
9696
scan->xs_ctup.t_data = NULL;
9797
scan->xs_cbuf = InvalidBuffer;
9898

99-
pgstat_initstats(&scan->xs_pgstat_info, indexRelation);
100-
10199
/*
102100
* Let the AM fill in the key and any opaque data it wants.
103101
*/

‎src/backend/access/index/indexam.c

+5-7
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.97 2007/01/05 22:19:23 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.98 2007/05/27 03:50:38 tgl Exp $
1212
*
1313
* INTERFACE ROUTINES
1414
* index_open - open an index relation by relation OID
@@ -145,8 +145,6 @@ index_open(Oid relationId, LOCKMODE lockmode)
145145
errmsg("\"%s\" is not an index",
146146
RelationGetRelationName(r))));
147147

148-
pgstat_initstats(&r->pgstat_info, r);
149-
150148
return r;
151149
}
152150

@@ -433,14 +431,14 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
433431
return NULL; /* failure exit */
434432
}
435433

436-
pgstat_count_index_tuples(&scan->xs_pgstat_info, 1);
434+
pgstat_count_index_tuples(scan->indexRelation, 1);
437435

438436
/*
439437
* Fetch the heap tuple and see if it matches the snapshot.
440438
*/
441439
if (heap_release_fetch(scan->heapRelation, scan->xs_snapshot,
442440
heapTuple, &scan->xs_cbuf, true,
443-
&scan->xs_pgstat_info))
441+
scan->indexRelation))
444442
break;
445443

446444
/* Skip if no undeleted tuple at this location */
@@ -502,7 +500,7 @@ index_getnext_indexitem(IndexScanDesc scan,
502500
Int32GetDatum(direction)));
503501

504502
if (found)
505-
pgstat_count_index_tuples(&scan->xs_pgstat_info, 1);
503+
pgstat_count_index_tuples(scan->indexRelation, 1);
506504

507505
return found;
508506
}
@@ -543,7 +541,7 @@ index_getmulti(IndexScanDesc scan,
543541
Int32GetDatum(max_tids),
544542
PointerGetDatum(returned_tids)));
545543

546-
pgstat_count_index_tuples(&scan->xs_pgstat_info, *returned_tids);
544+
pgstat_count_index_tuples(scan->indexRelation, *returned_tids);
547545

548546
return found;
549547
}

‎src/backend/access/nbtree/nbtsearch.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/nbtree/nbtsearch.c,v 1.112 2007/04/06 22:33:42 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/nbtree/nbtsearch.c,v 1.113 2007/05/27 03:50:39 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -453,7 +453,7 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
453453
int i;
454454
StrategyNumber strat_total;
455455

456-
pgstat_count_index_scan(&scan->xs_pgstat_info);
456+
pgstat_count_index_scan(rel);
457457

458458
/*
459459
* Examine the scan keys and eliminate any redundant keys; also mark the

‎src/backend/access/transam/twophase.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.30 2007/04/30 21:01:52 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.31 2007/05/27 03:50:39 tgl Exp $
1111
*
1212
* NOTES
1313
* Each global transaction is associated with a global transaction
@@ -1211,7 +1211,8 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
12111211
else
12121212
ProcessRecords(bufptr, xid, twophase_postabort_callbacks);
12131213

1214-
pgstat_count_xact_commit();
1214+
/* Count the prepared xact as committed or aborted */
1215+
AtEOXact_PgStat(isCommit);
12151216

12161217
/*
12171218
* And now we can clean up our mess.

‎src/backend/access/transam/twophase_rmgr.c

+8-4
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,15 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/transam/twophase_rmgr.c,v 1.4 2007/01/05 22:19:23 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/transam/twophase_rmgr.c,v 1.5 2007/05/27 03:50:39 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515
#include "postgres.h"
1616

1717
#include "access/twophase_rmgr.h"
1818
#include "commands/async.h"
19+
#include "pgstat.h"
1920
#include "storage/lock.h"
2021
#include "utils/flatfiles.h"
2122
#include "utils/inval.h"
@@ -27,7 +28,8 @@ const TwoPhaseCallback twophase_recover_callbacks[TWOPHASE_RM_MAX_ID + 1] =
2728
lock_twophase_recover, /* Lock */
2829
NULL, /* Inval */
2930
NULL, /* flat file update */
30-
NULL /* notify/listen */
31+
NULL, /* notify/listen */
32+
NULL /* pgstat */
3133
};
3234

3335
const TwoPhaseCallback twophase_postcommit_callbacks[TWOPHASE_RM_MAX_ID + 1] =
@@ -36,7 +38,8 @@ const TwoPhaseCallback twophase_postcommit_callbacks[TWOPHASE_RM_MAX_ID + 1] =
3638
lock_twophase_postcommit, /* Lock */
3739
inval_twophase_postcommit, /* Inval */
3840
flatfile_twophase_postcommit, /* flat file update */
39-
notify_twophase_postcommit /* notify/listen */
41+
notify_twophase_postcommit, /* notify/listen */
42+
pgstat_twophase_postcommit /* pgstat */
4043
};
4144

4245
const TwoPhaseCallback twophase_postabort_callbacks[TWOPHASE_RM_MAX_ID + 1] =
@@ -45,5 +48,6 @@ const TwoPhaseCallback twophase_postabort_callbacks[TWOPHASE_RM_MAX_ID + 1] =
4548
lock_twophase_postabort, /* Lock */
4649
NULL, /* Inval */
4750
NULL, /* flat file update */
48-
NULL /* notify/listen */
51+
NULL, /* notify/listen */
52+
pgstat_twophase_postabort /* pgstat */
4953
};

0 commit comments

Comments
 (0)