Re: postmaster uses more CPU in 18 beta1 with io_method=io_uring

From: Andres Freund <andres(at)anarazel(dot)de>
To: "Burd, Greg" <greg(at)burd(dot)me>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, MARK CALLAGHAN <mdcallag(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Tomas Vondra <tomas(at)vondra(dot)me>, Nathan Bossart <nathandbossart(at)gmail(dot)com>
Subject: Re: postmaster uses more CPU in 18 beta1 with io_method=io_uring
Date: 2025-07-08 00:04:25
Message-ID: ldmkngbpa5ys57tmojtelq7ut7pfqz3stq3nmstnfia5rjvplv@lplzzfgfmleu
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2025-06-30 15:31:14 -0400, Burd, Greg wrote:
> > On Jun 30, 2025, at 12:27 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > On 2025-06-05 14:32:10 -0400, Andres Freund wrote:
> >> On 2025-06-05 12:47:52 -0400, Tom Lane wrote:
> >>> Andres Freund <andres(at)anarazel(dot)de> writes:
> >>>> I think this is a big enough pitfall that it's, obviously assuming the patch
> >>>> has a sensible complexity, worth fixing this in 18. RMT, anyone, what do you
> >>>> think?
> >>>
> >>> Let's see the patch ... but yeah, I'd rather not ship 18 like this.
> >>
> >> I've attached a first draft.
> >>
> >> I can't make heads or tails of the ordering in configure.ac, so the function
> >> test is probably in the wrong place.
> >
> > Any comments on that patch? I'd hoped for some review comments... Unless I'll
> > hear otherwise, I'll just do a bit more polish and push..
>
> Thanks for doing this work!
>
> I just read through the v1 patch and it looks good. I have just a few small nit-picky questions:
>
> + #if defined(HAVE_LIBURING_QUEUE_INIT_MEM) && defined(IORING_SETUP_NO_MMAP) && 1
>
> The '1' looks like cruft, or am I missing something?

It's for making it easy to test both paths when running on an kernel/liburing
combo that's new enought o have support.

> + /* FIXME: This should probably not stay at DEBUG1? */
>
> Worth fixing before pushing?

Yes. I was just not yet sure what it should be. I ended up concluding that
it's probably fine to just keep it at DEBUG1...

> Also, this returns 'Size' but in the function uses 'size_t' I assume that's intentional?
>
> + static Size
> + pgaio_uring_ring_shmem_size(void)
>
> The next, similar, function below this one returns 'size_t'.

You're right - I wish we would just do a (slightly smarter) version of
s/Size/size_t/...

> Finally, and this may be me missing something everyone else knows is convention.
>
> + * XXX: We allocate memory for all PgAioUringContext instances and, if
>
> Is there any reason to keep the 'XXX'? You ask yourself a question in that
> comment, do you know the answer or was that a request to reviewers for
> feedback? :)

A bit of both :). I concluded that it's not worth having a separate segment,
there's not enough memory here to matter...

> I hope that is helpful.

Yep!

Greetings,

Andres Freund

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2025-07-08 00:11:55 Re: [WIP]Vertical Clustered Index (columnar store extension) - take2
Previous Message Vik Fearing 2025-07-08 00:03:29 Re: What is a typical precision of gettimeofday()?