Re: Automatically sizing the IO worker pool

Hi,

On 2026-04-08 14:09:16 +1200, Thomas Munro wrote:
> On Wed, Apr 8, 2026 at 12:30 PM Andres Freund <andres@anarazel.de> 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