Optimising compactify_tuples()

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: Optimising compactify_tuples()

Peter Geoghegan-4
On Tue, Sep 15, 2020 at 7:01 PM Thomas Munro <[hidden email]> wrote:
> On Wed, Sep 16, 2020 at 1:30 PM David Rowley <[hidden email]> wrote:
> > I also did some further performance tests of something other than
> > recovery. I can also report a good performance improvement in VACUUM.
> > Something around the ~25% reduction mark
>
> Wonderful results.  It must be rare for a such a localised patch to
> have such a large effect on such common workloads.

Yes, that's terrific.

--
Peter Geoghegan


Reply | Threaded
Open this post in threaded view
|

Re: Optimising compactify_tuples()

Simon Riggs
In reply to this post by David Rowley
On Thu, 10 Sep 2020 at 14:45, David Rowley <[hidden email]> wrote:

> I've also attached another tiny patch that I think is pretty useful
> separate from this. It basically changes:
>
> LOG:  redo done at 0/D518FFD0
>
> into:
>
> LOG:  redo done at 0/D518FFD0 system usage: CPU: user: 58.93 s,
> system: 0.74 s, elapsed: 62.31 s
>
> (I was getting sick of having to calculate the time spent from the log
> timestamps.)

I really like this patch, thanks for proposing it.

Should pg_rusage_init(&ru0);
be at the start of the REDO loop, since you only use it if we take that path?

--
Simon Riggs                http://www.2ndQuadrant.com/
Mission Critical Databases


Reply | Threaded
Open this post in threaded view
|

Re: Optimising compactify_tuples()

Robert Haas
On Wed, Sep 16, 2020 at 2:54 PM Simon Riggs <[hidden email]> wrote:
> I really like this patch, thanks for proposing it.

I'm pleased to be able to say that I agree completely with this
comment from Simon. :-)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Reply | Threaded
Open this post in threaded view
|

Re: Optimising compactify_tuples()

Andres Freund
In reply to this post by Thomas Munro-5
On 2020-09-16 14:01:21 +1200, Thomas Munro wrote:

> On Wed, Sep 16, 2020 at 1:30 PM David Rowley <[hidden email]> wrote:
> > Thanks a lot for the detailed benchmark results and profiles. That was
> > useful.  I've pushed both patches now. I did a bit of a sweep of the
> > comments on the 0001 patch before pushing it.
> >
> > I also did some further performance tests of something other than
> > recovery. I can also report a good performance improvement in VACUUM.
> > Something around the ~25% reduction mark
>
> Wonderful results.  It must be rare for a such a localised patch to
> have such a large effect on such common workloads.

Indeed!


Reply | Threaded
Open this post in threaded view
|

Re: Optimising compactify_tuples()

David Rowley
In reply to this post by Simon Riggs
Hi Simon,

On Thu, 17 Sep 2020 at 06:54, Simon Riggs <[hidden email]> wrote:
> Should pg_rusage_init(&ru0);
> be at the start of the REDO loop, since you only use it if we take that path?

Thanks for commenting.

I may be misunderstanding your words, but as far as I see it the
pg_rusage_init() is only called if we're going to go into recovery.
The pg_rusage_init() and pg_rusage_show() seem to be in the same
scope, so I can't quite see how we could do the pg_rusage_init()
without the pg_rusage_show().  Oh wait, there's the possibility that
if recoveryTargetAction == RECOVERY_TARGET_ACTION_SHUTDOWN that we'll
exit before we report end of recovery.  I'm pretty sure I'm
misunderstanding you though.

If it's easier to explain, please just post a small patch with what you mean.

David


12