public inbox for [email protected]
help / color / mirror / Atom feedFrom: Drouvot, Bertrand <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry
Date: Wed, 15 Feb 2023 09:21:48 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
Hi,
On 2/15/23 1:56 AM, Kyotaro Horiguchi wrote:
> At Tue, 14 Feb 2023 15:43:26 +0100, "Drouvot, Bertrand" <[email protected]> wrote in
>
> The comment assumed that the function directly returns an entry from
> shared memory, but now it copies the entry's contents into a palloc'ed
> memory and stores the sums of some counters for the current
> transaction in it. Do you think we should update the comment to
> reflect this change?
>
Good point, thanks! Yeah, definitively, done in V5 attached.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index f793ac1516..b26e2a5a7a 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -471,20 +471,46 @@ pgstat_fetch_stat_tabentry_ext(bool shared, Oid reloid)
* Find any existing PgStat_TableStatus entry for rel_id in the current
* database. If not found, try finding from shared tables.
*
+ * If an entry is found, copy it and increment the copy's counters with their
+ * subtransactions counterparts. Then return the copy. There is no need for the
+ * caller to pfree the copy as the MemoryContext will be reset soon after.
+ *
* If no entry found, return NULL, don't create a new one
*/
PgStat_TableStatus *
find_tabstat_entry(Oid rel_id)
{
PgStat_EntryRef *entry_ref;
+ PgStat_TableXactStatus *trans;
+ PgStat_TableStatus *tabentry = NULL;
+ PgStat_TableStatus *tablestatus = NULL;
entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_RELATION, MyDatabaseId, rel_id);
if (!entry_ref)
+ {
entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_RELATION, InvalidOid, rel_id);
+ if(!entry_ref)
+ return tablestatus;
+ }
+
+ tabentry = (PgStat_TableStatus *) entry_ref->pending;
+ tablestatus = palloc(sizeof(PgStat_TableStatus));
+ *tablestatus = *tabentry;
+
+ /*
+ * Live subtransactions' counts aren't in t_counts yet. This is not a hot
+ * code path so it sounds ok to reconcile for tuples_inserted,
+ * tuples_updated and tuples_deleted even if this is not what the caller
+ * is interested in.
+ */
+ for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
+ {
+ tablestatus->t_counts.t_tuples_inserted += trans->tuples_inserted;
+ tablestatus->t_counts.t_tuples_updated += trans->tuples_updated;
+ tablestatus->t_counts.t_tuples_deleted += trans->tuples_deleted;
+ }
- if (entry_ref)
- return entry_ref->pending;
- return NULL;
+ return tablestatus;
}
/*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9d707c3521..405d080daa 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1548,17 +1548,11 @@ pg_stat_get_xact_tuples_inserted(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
int64 result;
PgStat_TableStatus *tabentry;
- PgStat_TableXactStatus *trans;
if ((tabentry = find_tabstat_entry(relid)) == NULL)
result = 0;
else
- {
- result = tabentry->t_counts.t_tuples_inserted;
- /* live subtransactions' counts aren't in t_tuples_inserted yet */
- for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
- result += trans->tuples_inserted;
- }
+ result = (int64) (tabentry->t_counts.t_tuples_inserted);
PG_RETURN_INT64(result);
}
@@ -1569,17 +1563,11 @@ pg_stat_get_xact_tuples_updated(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
int64 result;
PgStat_TableStatus *tabentry;
- PgStat_TableXactStatus *trans;
if ((tabentry = find_tabstat_entry(relid)) == NULL)
result = 0;
else
- {
- result = tabentry->t_counts.t_tuples_updated;
- /* live subtransactions' counts aren't in t_tuples_updated yet */
- for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
- result += trans->tuples_updated;
- }
+ result = (int64) (tabentry->t_counts.t_tuples_updated);
PG_RETURN_INT64(result);
}
@@ -1590,17 +1578,11 @@ pg_stat_get_xact_tuples_deleted(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
int64 result;
PgStat_TableStatus *tabentry;
- PgStat_TableXactStatus *trans;
if ((tabentry = find_tabstat_entry(relid)) == NULL)
result = 0;
else
- {
- result = tabentry->t_counts.t_tuples_deleted;
- /* live subtransactions' counts aren't in t_tuples_deleted yet */
- for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
- result += trans->tuples_deleted;
- }
+ result = (int64) (tabentry->t_counts.t_tuples_deleted);
PG_RETURN_INT64(result);
}
Attachments:
[text/plain] v5-0001-find_tabstat_entry_recon.patch (3.9K, ../[email protected]/2-v5-0001-find_tabstat_entry_recon.patch)
download | inline diff:
diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index f793ac1516..b26e2a5a7a 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -471,20 +471,46 @@ pgstat_fetch_stat_tabentry_ext(bool shared, Oid reloid)
* Find any existing PgStat_TableStatus entry for rel_id in the current
* database. If not found, try finding from shared tables.
*
+ * If an entry is found, copy it and increment the copy's counters with their
+ * subtransactions counterparts. Then return the copy. There is no need for the
+ * caller to pfree the copy as the MemoryContext will be reset soon after.
+ *
* If no entry found, return NULL, don't create a new one
*/
PgStat_TableStatus *
find_tabstat_entry(Oid rel_id)
{
PgStat_EntryRef *entry_ref;
+ PgStat_TableXactStatus *trans;
+ PgStat_TableStatus *tabentry = NULL;
+ PgStat_TableStatus *tablestatus = NULL;
entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_RELATION, MyDatabaseId, rel_id);
if (!entry_ref)
+ {
entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_RELATION, InvalidOid, rel_id);
+ if(!entry_ref)
+ return tablestatus;
+ }
+
+ tabentry = (PgStat_TableStatus *) entry_ref->pending;
+ tablestatus = palloc(sizeof(PgStat_TableStatus));
+ *tablestatus = *tabentry;
+
+ /*
+ * Live subtransactions' counts aren't in t_counts yet. This is not a hot
+ * code path so it sounds ok to reconcile for tuples_inserted,
+ * tuples_updated and tuples_deleted even if this is not what the caller
+ * is interested in.
+ */
+ for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
+ {
+ tablestatus->t_counts.t_tuples_inserted += trans->tuples_inserted;
+ tablestatus->t_counts.t_tuples_updated += trans->tuples_updated;
+ tablestatus->t_counts.t_tuples_deleted += trans->tuples_deleted;
+ }
- if (entry_ref)
- return entry_ref->pending;
- return NULL;
+ return tablestatus;
}
/*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9d707c3521..405d080daa 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1548,17 +1548,11 @@ pg_stat_get_xact_tuples_inserted(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
int64 result;
PgStat_TableStatus *tabentry;
- PgStat_TableXactStatus *trans;
if ((tabentry = find_tabstat_entry(relid)) == NULL)
result = 0;
else
- {
- result = tabentry->t_counts.t_tuples_inserted;
- /* live subtransactions' counts aren't in t_tuples_inserted yet */
- for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
- result += trans->tuples_inserted;
- }
+ result = (int64) (tabentry->t_counts.t_tuples_inserted);
PG_RETURN_INT64(result);
}
@@ -1569,17 +1563,11 @@ pg_stat_get_xact_tuples_updated(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
int64 result;
PgStat_TableStatus *tabentry;
- PgStat_TableXactStatus *trans;
if ((tabentry = find_tabstat_entry(relid)) == NULL)
result = 0;
else
- {
- result = tabentry->t_counts.t_tuples_updated;
- /* live subtransactions' counts aren't in t_tuples_updated yet */
- for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
- result += trans->tuples_updated;
- }
+ result = (int64) (tabentry->t_counts.t_tuples_updated);
PG_RETURN_INT64(result);
}
@@ -1590,17 +1578,11 @@ pg_stat_get_xact_tuples_deleted(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
int64 result;
PgStat_TableStatus *tabentry;
- PgStat_TableXactStatus *trans;
if ((tabentry = find_tabstat_entry(relid)) == NULL)
result = 0;
else
- {
- result = tabentry->t_counts.t_tuples_deleted;
- /* live subtransactions' counts aren't in t_tuples_deleted yet */
- for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
- result += trans->tuples_deleted;
- }
+ result = (int64) (tabentry->t_counts.t_tuples_deleted);
PG_RETURN_INT64(result);
}
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox