Hi, On 2025-09-15 18:21:22 -0400, Andres Freund wrote: > On September 15, 2025 6:11:34 PM EDT, Sami Imseih <[email protected]> wrote: > >I have been reviewing how custom cumulative statistics should update their > >counters, and noticed something unexpected in the injection_points example > >[0]. > > > >In pgstat_report_inj(), the code updates shared_stats directly: > > > >``` > >shstatent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats; > >statent = &shstatent->stats; > > > >/* Update the injection point statistics */ > >statent->numcalls++; > >``` > > > >This works because it increments shared memory directly, but it bypasses the > >usual pattern where updates go into ->pending and are later flushed into > >shared memory by .flush_pending_cb > > > >I think the more appropriate way to do this is: > > > >``` > > pgstat_report_inj(const char *name) > > { > > PgStat_EntryRef *entry_ref; > >- PgStatShared_InjectionPoint *shstatent; > > PgStat_StatInjEntry *statent; > > > > /* leave if disabled */ > >@@ -174,8 +173,7 @@ pgstat_report_inj(const char *name) > > entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_INJECTION, > > InvalidOid, > > > > PGSTAT_INJ_IDX(name), NULL); > > > >- shstatent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats; > >- statent = &shstatent->stats; > >+ statent = (PgStat_StatInjEntry *) entry_ref->pending; > > > > /* Update the injection point statistics */ > > statent->numcalls++; > >``` > > > >which for example pgstat_report_subscription_error does something > >similar. > > > >Maybe I am missing something obvious for injection_points? > > The point of deferring updating shared stats is to avoid contention. That's > certainly crucial for something like relation access. But it seems extremely > unlikely that there would be contention due to injection point stat updates.
But, uh, the code is incorrect as-is, due to not locking the shared stats while updating them. Which means it's entirely possible to loose stats updates if updated by multipole backends. Am I missing something? Greetings, Andres Freund
