At Thu, 24 Mar 2022 13:21:33 -0400, Melanie Plageman <melanieplage...@gmail.com> wrote in > On Thu, Mar 17, 2022 at 3:36 AM Andres Freund <and...@anarazel.de> 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