MIME-Version: 1.0
References: <aXcQ0Bdff5OsUDWY@ip-10-97-1-34.eu-west-3.compute.internal>
 <aXmuKZz8sXPLCumT@paquier.xyz> <aXnveIxQEB5wwcB8@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAA5RZ0tGhu9jXGgFjzx7yK4cseE+t40Q4qZ+KhY66B2MZ=pBrQ@mail.gmail.com>
 <CAA5RZ0tvxZTZ7Xwm7FjWxAY_OphCq7tmpCaLgas0qrh+J86pRg@mail.gmail.com>
 <aYDb7g5hXnaP+5BU@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAA5RZ0tgr5iFuBDofX9n4SNbo-SFjzNFqTGHt7yQFAmfWBf-Rw@mail.gmail.com>
 <CAA5RZ0sqJzpBZnOY3rOy5x7+WL_UDnoMkn2S_ZoXPPOWZkXSbA@mail.gmail.com>
 <aYXL6XmVABORjGF5@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAA5RZ0tK=-Dukr4ofmKpPSK4-LwqrGXmzkqGoMuJKPXnGW3=AA@mail.gmail.com> <aZLVDJLTiFlxsWZi@ip-10-97-1-34.eu-west-3.compute.internal>
In-Reply-To: <aZLVDJLTiFlxsWZi@ip-10-97-1-34.eu-west-3.compute.internal>
From: Sami Imseih <samimseih@gmail.com>
Date: Mon, 16 Feb 2026 16:42:50 -0600
Message-ID: <CAA5RZ0tPsU_bCATn-Wtf8hMaKmrCwfxtLcY9Pp3NQPpLrH2G_Q@mail.gmail.com>
Subject: Re: Flush some statistics within running transactions
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Fujii Masao <masao.fujii@gmail.com>, 
	pgsql-hackers@lists.postgresql.org, Zsolt Parragi <zsolt.parragi@percona.com>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAA5RZ0tPsU_bCATn-Wtf8hMaKmrCwfxtLcY9Pp3NQPpLrH2G_Q%40mail.gmail.com>
Precedence: bulk

Thanks!

> > 2/
> > I don't see we have tests for other timeout based GUCs, but it would nice
> > to ensure that this woks correctly. Maybe as a custom_stats test where we
> > SET stats_flush_interval inside the transaction and make sure the stats flush
> > only after the new timeout has passed. Maybe?
>
> Not sure I follow, that's what 0002 is doing.

I was referring to "SET stats_flush_interval", which you are doing in 0005,
so disregard this comment. I wrote this comment when I read 0003 only.


> > Will this be tue at all time? Let's imagine a Kind that flushes all the fields
> > ANYTIME, would we not want to delete the pending entry?
> >
> > +               /* if successfull non-partial flush, remove entry */
> > +               if (did_flush && !is_partial_flush)
> >                         pgstat_delete_pending_entry(entry_ref);
> >
>
> Right, and that's why the MIXED flush mode was useful, i.e to be able to distinguish
> here. So, in the attached, instead of re-introducing the MIXED flush mode, I added
> a "is_partial" bool paramater to the flush_pending_cb().
>

Yeah, MIXED flush mode to deal with this is still not the right idea, IMO.

I do agree with what you did here by passing "is_partial" to the callbacks.
Now it will be up to the callbacks to determine if they performed a partial
or a complete flush ( even in the case of any anytime flush, if they so
happened to flush stats for all the fields ).

I do not have any further comments on this patchset.

--
Sami Imseih
Amazon Web Services (AWS)