Feedback-ID: id4a34324:Fastmail
Date: Thu, 17 Nov 2022 08:24:31 -0800
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	"Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>,
	Nitin Jadhav <nitinjadhavpostgres@gmail.com>,
	Matthias van de Meent <boekewurm+postgres@gmail.com>,
	Ashutosh Sharma <ashu.coek88@gmail.com>,
	Julien Rouhaud <rjuju123@gmail.com>,
	Bruce Momjian <bruce@momjian.us>, Tom Lane <tgl@sss.pgh.pa.us>,
	Magnus Hagander <magnus@hagander.net>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Report checkpoint progress with pg_stat_progress_checkpoint
 (was: Report checkpoint progress in server logs)
Message-ID: <20221117162431.a3kqnpgjjsa7foxy@awork3.anarazel.de>
References: <20220319001556.atuqbmafvc73pa2k@alap3.anarazel.de>
 <CAMm1aWa_3N1Z2-_YwdNxfogs_rxzpu90VZeKjqwjerCVq9rQAw@mail.gmail.com>
 <20220707000446.hefgyu5xikxwt4md@alap3.anarazel.de>
 <CAMm1aWbfFwT+Cf1eSrXtF2R8ffxXTbH=un8unQ8WbcpcbymnYw@mail.gmail.com>
 <d9546a13-dd86-57d8-3bb4-32136fbb3673@gmail.com>
 <CA+TgmoaJEDirAMq_9q3B8PrXDf1h72S7hhPr4aotakYe9HLoLw@mail.gmail.com>
 <CALj2ACX5xK_X=MSBh_Qe518O7V5D2phnpXuSD9=-zAkGnyo9vA@mail.gmail.com>
 <CA+TgmoYCu6RpuJ3cZz0e7cZSfaVb=cr6iVcgGMGd5dxX0MYNRA@mail.gmail.com>
 <20221116195232.wynvnlng2ivne76b@awork3.anarazel.de>
 <CA+TgmoYJ9qdK6naA2pxMZc+amyNCSRScBg4W-Anqd_8y2oZt0A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoYJ9qdK6naA2pxMZc+amyNCSRScBg4W-Anqd_8y2oZt0A@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20221117162431.a3kqnpgjjsa7foxy%40awork3.anarazel.de>
Precedence: bulk

Hi,

On 2022-11-17 09:03:32 -0500, Robert Haas wrote:
> On Wed, Nov 16, 2022 at 2:52 PM Andres Freund <andres@anarazel.de> wrote:
> > I don't think we'd want every buffer write or whatnot go through the
> > changecount mechanism, on some non-x86 platforms that could be noticable. But
> > if we didn't stage the stats updates locally I think we could make most of the
> > stats changes without that overhead.  For updates that just increment a single
> > counter there's simply no benefit in the changecount mechanism afaict.
>
> You might be right, but I'm not sure whether it's worth stressing
> about. The progress reporting mechanism uses the st_changecount
> mechanism, too, and as far as I know nobody's complained about that
> having too much overhead. Maybe they have, though, and I've just
> missed it.

I've seen it in profiles, although not as the major contributor. Most places
do a reasonable amount of work between calls though.

As an experiment, I added a progress report to BufferSync()'s first loop
(i.e. where it checks all buffers). On a 128GB shared_buffers cluster that
increases the time for a do-nothing checkpoint from ~235ms to ~280ms. If I
remove the changecount stuff and use a single write + write barrier, it ends
up as 250ms. Inlining brings it down a bit further, to 247ms.

Obviously this is a very extreme case - we only do very little work between
the progress report calls. But it does seem to show that the overhead is not
entirely neglegible.


I think pgstat_progress_start_command() needs the changecount stuff, as does
pgstat_progress_update_multi_param(). But for anything updating a single
parameter at a time it really doesn't do anything useful on a platform that
doesn't tear 64bit writes (so it could be #ifdef
PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY).


Out of further curiosity I wanted to test the impact when the loop doesn't
even do a LockBufHdr() and added an unlocked pre-check. 109ms without
progress. 138ms with. 114ms with the simplified
pgstat_progress_update_param(). 108ms after inlining the simplified
pgstat_progress_update_param().

Greetings,

Andres Freund