Hi,
On 11/18/22 7:06 AM, Bharath Rupireddy wrote:
On Fri, Nov 18, 2022 at 10:32 AM Drouvot, Bertrand
<bertranddrouvot...@gmail.com> wrote:
Hi hackers,
Please find attached a patch proposal to avoid 2 calls to
pgstat_fetch_stat_tabentry_ext() in pgstat_fetch_stat_tabentry() in case
the relation is not a shared one and no statistics are found.
Thanks Andres for the suggestion done in [1].
[1]:
https://www.postgresql.org/message-id/20221116201202.3k74ajawyom2c3eq%40awork3.anarazel.de
+1. The patch LGTM.
Thanks for looking at it!
However, I have a suggestion to simplify it
further by getting rid of the local variable tabentry and just
returning pgstat_fetch_stat_tabentry_ext(IsSharedRelation(relid),
relid);. Furthermore, the pgstat_fetch_stat_tabentry() can just be a
static inline function.
Good point. While at it, why not completely get rid of
pgstat_fetch_stat_tabentry_ext(), like in v2 the attached?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/postmaster/autovacuum.c
b/src/backend/postmaster/autovacuum.c
index 601834d4b4..fde8458f5e 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2101,8 +2101,8 @@ do_autovacuum(void)
/* Fetch reloptions and the pgstat entry for this table */
relopts = extract_autovac_opts(tuple, pg_class_desc);
- tabentry =
pgstat_fetch_stat_tabentry_ext(classForm->relisshared,
-
relid);
+ tabentry = pgstat_fetch_stat_tabentry(classForm->relisshared,
+
relid);
/* Check if it needs vacuum or analyze */
relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
@@ -2185,8 +2185,8 @@ do_autovacuum(void)
}
/* Fetch the pgstat entry for this table */
- tabentry =
pgstat_fetch_stat_tabentry_ext(classForm->relisshared,
-
relid);
+ tabentry = pgstat_fetch_stat_tabentry(classForm->relisshared,
+
relid);
relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
effective_multixact_freeze_max_age,
@@ -2913,8 +2913,8 @@ recheck_relation_needs_vacanalyze(Oid relid,
PgStat_StatTabEntry *tabentry;
/* fetch the pgstat table entry */
- tabentry = pgstat_fetch_stat_tabentry_ext(classForm->relisshared,
-
relid);
+ tabentry = pgstat_fetch_stat_tabentry(classForm->relisshared,
+
relid);
relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
effective_multixact_freeze_max_age,
diff --git a/src/backend/utils/activity/pgstat_relation.c
b/src/backend/utils/activity/pgstat_relation.c
index 55a355f583..266fa0c15e 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -61,8 +61,8 @@ pgstat_copy_relation_stats(Relation dst, Relation src)
PgStatShared_Relation *dstshstats;
PgStat_EntryRef *dst_ref;
- srcstats = pgstat_fetch_stat_tabentry_ext(src->rd_rel->relisshared,
-
RelationGetRelid(src));
+ srcstats = pgstat_fetch_stat_tabentry(src->rd_rel->relisshared,
+
RelationGetRelid(src));
if (!srcstats)
return;
@@ -435,27 +435,7 @@ pgstat_update_heap_dead_tuples(Relation rel, int delta)
* caller is better off to report ZERO instead.
*/
PgStat_StatTabEntry *
-pgstat_fetch_stat_tabentry(Oid relid)
-{
- PgStat_StatTabEntry *tabentry;
-
- tabentry = pgstat_fetch_stat_tabentry_ext(false, relid);
- if (tabentry != NULL)
- return tabentry;
-
- /*
- * If we didn't find it, maybe it's a shared table.
- */
- tabentry = pgstat_fetch_stat_tabentry_ext(true, relid);
- return tabentry;
-}
-
-/*
- * More efficient version of pgstat_fetch_stat_tabentry(), allowing to specify
- * whether the to-be-accessed table is a shared relation or not.
- */
-PgStat_StatTabEntry *
-pgstat_fetch_stat_tabentry_ext(bool shared, Oid reloid)
+pgstat_fetch_stat_tabentry(bool shared, Oid reloid)
{
Oid dboid = (shared ? InvalidOid : MyDatabaseId);
diff --git a/src/backend/utils/adt/pgstatfuncs.c
b/src/backend/utils/adt/pgstatfuncs.c
index ae3365d917..8eed606702 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -31,6 +31,7 @@
#include "utils/builtins.h"
#include "utils/inet.h"
#include "utils/timestamp.h"
+#include "catalog/catalog.h"
#define UINT32_ACCESS_ONCE(var) ((uint32)(*((volatile uint32
*)&(var))))
@@ -43,7 +44,7 @@ pg_stat_get_numscans(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->numscans);
@@ -59,7 +60,7 @@ pg_stat_get_lastscan(PG_FUNCTION_ARGS)
TimestampTz result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = tabentry->lastscan;
@@ -78,7 +79,7 @@ pg_stat_get_tuples_returned(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->tuples_returned);
@@ -94,7 +95,7 @@ pg_stat_get_tuples_fetched(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->tuples_fetched);
@@ -110,7 +111,7 @@ pg_stat_get_tuples_inserted(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->tuples_inserted);
@@ -126,7 +127,7 @@ pg_stat_get_tuples_updated(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->tuples_updated);
@@ -142,7 +143,7 @@ pg_stat_get_tuples_deleted(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->tuples_deleted);
@@ -158,7 +159,7 @@ pg_stat_get_tuples_hot_updated(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->tuples_hot_updated);
@@ -174,7 +175,7 @@ pg_stat_get_live_tuples(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->n_live_tuples);
@@ -190,7 +191,7 @@ pg_stat_get_dead_tuples(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->n_dead_tuples);
@@ -206,7 +207,7 @@ pg_stat_get_mod_since_analyze(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->changes_since_analyze);
@@ -222,7 +223,7 @@ pg_stat_get_ins_since_vacuum(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->inserts_since_vacuum);
@@ -238,7 +239,7 @@ pg_stat_get_blocks_fetched(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->blocks_fetched);
@@ -254,7 +255,7 @@ pg_stat_get_blocks_hit(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->blocks_hit);
@@ -269,7 +270,7 @@ pg_stat_get_last_vacuum_time(PG_FUNCTION_ARGS)
TimestampTz result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = tabentry->vacuum_timestamp;
@@ -287,7 +288,7 @@ pg_stat_get_last_autovacuum_time(PG_FUNCTION_ARGS)
TimestampTz result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = tabentry->autovac_vacuum_timestamp;
@@ -305,7 +306,7 @@ pg_stat_get_last_analyze_time(PG_FUNCTION_ARGS)
TimestampTz result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = tabentry->analyze_timestamp;
@@ -323,7 +324,7 @@ pg_stat_get_last_autoanalyze_time(PG_FUNCTION_ARGS)
TimestampTz result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = tabentry->autovac_analyze_timestamp;
@@ -341,7 +342,7 @@ pg_stat_get_vacuum_count(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->vacuum_count);
@@ -356,7 +357,7 @@ pg_stat_get_autovacuum_count(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->autovac_vacuum_count);
@@ -371,7 +372,7 @@ pg_stat_get_analyze_count(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->analyze_count);
@@ -386,7 +387,7 @@ pg_stat_get_autoanalyze_count(PG_FUNCTION_ARGS)
int64 result;
PgStat_StatTabEntry *tabentry;
- if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ if ((tabentry = pgstat_fetch_stat_tabentry(IsSharedRelation(relid),
relid)) == NULL)
result = 0;
else
result = (int64) (tabentry->autovac_analyze_count);
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 9e2ce6f011..10ae025988 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -569,9 +569,8 @@ extern void pgstat_twophase_postcommit(TransactionId xid,
uint16 info,
extern void pgstat_twophase_postabort(TransactionId xid, uint16 info,
void
*recdata, uint32 len);
-extern PgStat_StatTabEntry *pgstat_fetch_stat_tabentry(Oid relid);
-extern PgStat_StatTabEntry *pgstat_fetch_stat_tabentry_ext(bool shared,
-
Oid reloid);
+extern PgStat_StatTabEntry *pgstat_fetch_stat_tabentry(bool shared,
+
Oid reloid);
extern PgStat_TableStatus *find_tabstat_entry(Oid rel_id);
