public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: Thomas Munro <[email protected]>
Cc: Dmitry Dolgov <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Automatically sizing the IO worker pool
Date: Tue, 7 Apr 2026 22:20:55 -0400
Message-ID: <v4e2phxr3n22lv6skhtczbhy3xbyqwxuy7pj7cdh6afmhwhrqb@k6cpnb2kjw6c> (raw)
In-Reply-To: <CA+hUKGK=vELXFXNj2L=vTkof6s_EQzTjYXXrUVwOOW0rahEfVg@mail.gmail.com>
References: <CA+hUKG+P80oR7yy5-67uHqwBWr9rux69BkQF9fwz=f0LuDn3Rw@mail.gmail.com>
	<CA+hUKGKgVhLVS4Pne8cTR0f=tE8GtF4-CvY2HMDOJEm7w0iXfA@mail.gmail.com>
	<20260328095204.5tsq5bldugeumrtf@erthalion>
	<CA+hUKGLUZ51zkxUoBvANcLdvz8DjS_85=6985HE8gLuNtBxX7Q@mail.gmail.com>
	<jp7lhn2wng5mlhn3jvn4t6nmr23l6bnim5af3qslnoguevfqqz@d7cvksm62j4j>
	<CA+hUKGLp_yPztSoQkmRe1kFk1M5JNr7b+07cc+46AVg=VoKC-Q@mail.gmail.com>
	<adaz4sd2x64uoei2nmd4pinvothusd7uwllnilq6msfp4wytrw@npedbcvg3llt>
	<CA+hUKGJi6L2ftuTiKw2W4AAah6Uivs4JGiq58A7MjePbXpc4BA@mail.gmail.com>
	<4vhrgxmx5w4cjr7vgegur3hbkuojt2iz23v4dqfypsgvl5bszi@xrxefkkcxdja>
	<CA+hUKGK=vELXFXNj2L=vTkof6s_EQzTjYXXrUVwOOW0rahEfVg@mail.gmail.com>

Hi,

On 2026-04-08 14:09:16 +1200, Thomas Munro wrote:
> On Wed, Apr 8, 2026 at 12:30 PM Andres Freund <[email protected]> wrote:
> > On 2026-04-08 11:18:51 +1200, Thomas Munro wrote:
> > > >                 /* Choose one worker to wake for this batch. */
> > > >                 if (worker == -1)
> > > >                         worker = pgaio_worker_choose_idle(-1);
> > >
> > > Well I didn't want to wake a worker if we'd failed to enqueue
> > > anything.
> >
> > I think it's worth waking up workers if there are idle ones and the queue is
> > full?
> 
> True, but I prefer to test nsync because there is another reason to break:

I don't follow.  What I was proposing is after the conditional lock
acquisition succeeded.  So is your nsync == 0 check.

> +/*
> + * Tell postmaster that we think a new worker is needed.
> + */
> +static void
> +pgaio_worker_request_grow(void)
> +{
> +	/*
> +	 * Suppress useless signaling if we already know that we're at the
> +	 * maximum.  This uses an unlocked read of nworkers, but that's OK for
> +	 * this heuristic purpose.
> +	 */
> +	if (io_worker_control->nworkers < io_max_workers)
>  	{
> -		io_worker_control->workers[i].latch = NULL;
> -		io_worker_control->workers[i].in_use = false;
> +		if (!io_worker_control->grow)
> +		{
> +			io_worker_control->grow = true;
> +			pg_memory_barrier();
> +
> +			/*
> +			 * If the postmaster has already been signaled, don't do it again
> +			 * until the postmaster clears this flag.  There is no point in
> +			 * repeated signals if grow is being set and cleared repeatedly
> +			 * while the postmaster is waiting for io_worker_launch_interval
> +			 * (which it applies even to canceled requests).
> +			 */
> +			if (!io_worker_control->grow_signal_sent)
> +			{
> +				io_worker_control->grow_signal_sent = true;
> +				pg_memory_barrier();
> +				SendPostmasterSignal(PMSIGNAL_IO_WORKER_GROW);
> +			}
> +		}
>  	}
>  }


I'd probbly use early returns to make it a bit more readable.



> +static bool
> +pgaio_worker_can_timeout(void)
> +{
> +	PgAioWorkerSet workerset;
> +
> +	/* Serialize against pool size changes. */
> +	LWLockAcquire(AioWorkerControlLock, LW_SHARED);
> +	workerset = io_worker_control->workerset;
> +	LWLockRelease(AioWorkerControlLock);
> +
> +	if (MyIoWorkerId != pgaio_workerset_get_highest(&workerset))
> +		return false;
> +
> +	if (MyIoWorkerId < io_min_workers)
> +		return false;
> +
> +	return true;
> +}

I guess I'd move the < io_min_workers to earlier so that you don't acquire the
lock if that'll return false anyway.


Greetings,

Andres Freund

view thread (24+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Automatically sizing the IO worker pool
  In-Reply-To: <v4e2phxr3n22lv6skhtczbhy3xbyqwxuy7pj7cdh6afmhwhrqb@k6cpnb2kjw6c>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox