public inbox for [email protected]  

help / color / mirror / Atom feed

From: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: shared-memory based stats collector - v66
Date: Fri, 25 Mar 2022 14:22:56 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAKRu_YsFzW3jsQUD7MVUY8c+87OiKW_Y93TsPAfLr3zi_jX9w@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CAAKRu_YsFzW3jsQUD7MVUY8c+87OiKW_Y93TsPAfLr3zi_jX9w@mail.gmail.com>

At Thu, 24 Mar 2022 13:21:33 -0400, Melanie Plageman <[email protected]> wrote in 
> On Thu, Mar 17, 2022 at 3:36 AM Andres Freund <[email protected]> wrote:
> >
> > The biggest todos are:
> > - Address all the remaining AFIXMEs and XXXs
> 
> Attached is a patch that addresses three of the existing AFIXMEs.

Thanks!

+		.reset_timestamp_cb = pgstat_shared_reset_timestamp_noop,

(I once misunderstood that the "shared" means shared memory area..)

The reset function is type-specific and it must be set.  So don't we
provide all to-be-required reset functions?


+	if (pgstat_shared_ref_get(kind, dboid, objoid, false, NULL))
+	{
+		Oid msg_oid = (kind == PGSTAT_KIND_DB) ? dboid : objoid;

Explicitly using PGSTAT_KIND_DB here is a kind of annoyance.  Since we
always give InvalidOid correctly as the parameters, and objoid alone
is not specific enough, do we warn using both dboid and objoid without
a special treat?

Concretely, I propose to do the following instead.

+	if (pgstat_shared_ref_get(kind, dboid, objoid, false, NULL))
+	{
+		ereport(WARNING,
+				errmsg("resetting existing stats for type %s, db=%d, oid=%d",
+                      pgstat_kind_info_for(kind)->name, dboid, objoid);				



+pgstat_pending_delete(PgStatSharedRef *shared_ref)
+{
+	void	   *pending_data = shared_ref->pending;
+	PgStatKind kind = shared_ref->shared_entry->key.kind;
+
+	Assert(pending_data != NULL);
+	Assert(!pgstat_kind_info_for(kind)->fixed_amount);
+
+	/* PGSTAT_KIND_TABLE has its own callback */
+	Assert(kind != PGSTAT_KIND_TABLE);
+

"kind" is used only in assertion, which requires PG_USED_FOR_ASSERTS_ONLY.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

view thread (24+ 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], [email protected], [email protected], [email protected]
  Subject: Re: shared-memory based stats collector - v66
  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