Horton

Thread

Commits

  1. Use placeholders and not GUC names in error message (autovacuum)

  1. [PATCH] Warn when io_min_workers exceeds io_max_workers

    Baji Shaik <baji.pgdev@gmail.com> — 2026-06-22T21:21:17Z

    Hi,

Setting io_min_workers to a value greater than io_max_workers is
silently accepted, but the minimum has no effect and the worker pool
never grows past io_max_workers. This can confuse users who expect at
least io_min_workers workers to always be running.

Compare with autovacuum which properly warns:

    WARNING:  "autovacuum_max_workers" (3) should be less than or equal
    to "autovacuum_worker_slots" (1)
    DETAIL:  The server will only start up to "autovacuum_worker_slots" (1)
    autovacuum workers at a given time.

The attached patch adds check_io_worker_gucs() in
storage/aio/method_worker.c that emits a WARNING similar to what
autovacuum does for autovacuum_worker_slots vs autovacuum_max_workers:

    WARNING:  "io_min_workers" (5) should be less than or equal to
              "io_max_workers" (3)
    DETAIL:  The I/O worker pool will not exceed "io_max_workers" (3)
             workers.

The check runs in IO worker 0 at startup and after each configuration
reload.

Thanks,
Baji Shaik

  2. Re: [PATCH] Warn when io_min_workers exceeds io_max_workers

    Tristan Partin <tristan@partin.io> — 2026-06-22T22:30:10Z

    On Mon Jun 22, 2026 at 9:21 PM UTC, Baji Shaik wrote:
> Hi,
>
> Setting io_min_workers to a value greater than io_max_workers is
> silently accepted, but the minimum has no effect and the worker pool
> never grows past io_max_workers. This can confuse users who expect at
> least io_min_workers workers to always be running.
>
> Compare with autovacuum which properly warns:
>
>     WARNING:  "autovacuum_max_workers" (3) should be less than or equal
>     to "autovacuum_worker_slots" (1)
>     DETAIL:  The server will only start up to "autovacuum_worker_slots" (1)
>     autovacuum workers at a given time.
>
> The attached patch adds check_io_worker_gucs() in
> storage/aio/method_worker.c that emits a WARNING similar to what
> autovacuum does for autovacuum_worker_slots vs autovacuum_max_workers:
>
>     WARNING:  "io_min_workers" (5) should be less than or equal to
>               "io_max_workers" (3)
>     DETAIL:  The I/O worker pool will not exceed "io_max_workers" (3)
>              workers.
>
> The check runs in IO worker 0 at startup and after each configuration
> reload.

Baji,

The code looks good. I wonder if we should add a test. I could not find 
a similar test for autovacuum for what it's worth.

-- 
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

  3. Re: [PATCH] Warn when io_min_workers exceeds io_max_workers

    Baji Shaik <baji.pgdev@gmail.com> — 2026-06-23T01:27:13Z

    On Mon, Jun 22, 2026 at 5:30 PM Tristan Partin <tristan@partin.io> wrote:

> The code looks good. I wonder if we should add a test. I could not find
> a similar test for autovacuum for what it's worth.
>

Thanks for the review, Tristan.

I checked and confirmed there's no existing test covering the
check_av_worker_gucs() in autovacuum either.  I'm happy to add a TAP
test if you think it's needed. Let me know and I will include it in v2.

Thanks,
Baji Shaik.

  4. Re: [PATCH] Warn when io_min_workers exceeds io_max_workers

    Michael Paquier <michael@paquier.xyz> — 2026-06-23T01:29:35Z

    On Mon, Jun 22, 2026 at 10:30:10PM +0000, Tristan Partin wrote:
> The code looks good. I wonder if we should add a test. I could not find 
> a similar test for autovacuum for what it's worth.

Hmm, okay.  Why not warning in this case, with a message showing up
only once in the worker with ID=0 at startup and reload.

Let's adjust the messages so as the GUC parameters are included as %s
in the error strings, and apply the same thing for the autovacuum
path.  A benefit is that we are then able to reduce the translation
work churn with only one message to translate for both.  Hence, I'd
suggest to reword the error messages of both as of:
"\"%s\" (%d) should be less than or equal to \"%s\" (%d)",
"guc_max_min", min_val, "guc_min_name", max_val
--
Michael

  5. Re: [PATCH] Warn when io_min_workers exceeds io_max_workers

    Baji Shaik <baji.pgdev@gmail.com> — 2026-06-23T02:47:06Z

    On Mon, Jun 22, 2026 at 8:29 PM Michael Paquier <michael@paquier.xyz> wrote:

> Let's adjust the messages so as the GUC parameters are included as %s
> in the error strings, and apply the same thing for the autovacuum
> path.  A benefit is that we are then able to reduce the translation
> work churn with only one message to translate for both.  Hence, I'd
> suggest to reword the error messages of both as of:
> "\"%s\" (%d) should be less than or equal to \"%s\" (%d)",
> "guc_max_min", min_val, "guc_min_name", max_val
>

Thanks for the suggestion, Michael.

Attached v2 with two patches:

0001 - Adds check_io_worker_gucs() with parameterized GUC names (%s)
0002 - Adjusts check_av_worker_gucs() to use the same format

Both now produce the same translatable errmsg pattern:
 "\"%s\" (%d) should be less than or equal to \"%s\" (%d)"

Thanks,
Baji Shaik.

  6. Re: [PATCH] Warn when io_min_workers exceeds io_max_workers

    Michael Paquier <michael@paquier.xyz> — 2026-06-23T04:03:21Z

    On Mon, Jun 22, 2026 at 09:47:06PM -0500, Baji Shaik wrote:
> Attached v2 with two patches:
> 
> 0001 - Adds check_io_worker_gucs() with parameterized GUC names (%s)
> 0002 - Adjusts check_av_worker_gucs() to use the same format
> 
> Both now produce the same translatable errmsg pattern:
>  "\"%s\" (%d) should be less than or equal to \"%s\" (%d)"

That looks sensible here.
--
Michael

  7. Re: [PATCH] Warn when io_min_workers exceeds io_max_workers

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-06-23T05:36:32Z

    At Tue, 23 Jun 2026 13:03:21 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Mon, Jun 22, 2026 at 09:47:06PM -0500, Baji Shaik wrote:
> > Attached v2 with two patches:
> > 
> > 0001 - Adds check_io_worker_gucs() with parameterized GUC names (%s)
> > 0002 - Adjusts check_av_worker_gucs() to use the same format
> > 
> > Both now produce the same translatable errmsg pattern:
> >  "\"%s\" (%d) should be less than or equal to \"%s\" (%d)"
> 
> That looks sensible here.

I have two minor comments.

As Michael suggested, using placeholders for the GUC names could make
similar messages easier to share in the future, which may also help
reduce the number of translation strings. I wouldn't consider it
necessary to adjust the existing
autovacuum_max_workers/autovacuum_worker_slots message as part of this
patch, though.

Also, "Re-check" in the second call-site comment feels a bit vague to
me, since the previous call is quite far away. Perhaps both call-site
comments could simply describe checking the relevant GUC values, one
at startup and the other after a configuration reload.  Regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  8. Re: [PATCH] Warn when io_min_workers exceeds io_max_workers

    Baji Shaik <baji.pgdev@gmail.com> — 2026-06-23T16:02:57Z

    On Tue, Jun 23, 2026 at 12:36 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:

> Also, "Re-check" in the second call-site comment feels a bit vague to
> me, since the previous call is quite far away. Perhaps both call-site
> comments could simply describe checking the relevant GUC values, one
> at startup and the other after a configuration reload.  Regards,
>

Thanks for the review.

Attached v3 with both comments updated to
/* Emit a WARNING if io_min_workers > io_max_workers. */
as suggested.

I have kept the autovacuum message change as 0002 since Michael
suggested unifying the format strings. If the consensus is to
leave the existing autovacuum message alone, I'm happy to drop it.

Thanks,
Baji Shaik.

  9. Re: [PATCH] Warn when io_min_workers exceeds io_max_workers

    Michael Paquier <michael@paquier.xyz> — 2026-06-23T22:13:41Z

    On Tue, Jun 23, 2026 at 02:36:32PM +0900, Kyotaro Horiguchi wrote:
> As Michael suggested, using placeholders for the GUC names could make
> similar messages easier to share in the future, which may also help
> reduce the number of translation strings. I wouldn't consider it
> necessary to adjust the existing
> autovacuum_max_workers/autovacuum_worker_slots message as part of this
> patch, though.

Yes.  A lot of messages of v19 should already be translated, I guess?
I don't think that this is something we have to do in v19, so let's
just align everything once v20 opens up (minus tweaks to the wording
of the strings).
--
Michael

  10. Re: [PATCH] Warn when io_min_workers exceeds io_max_workers

    Michael Paquier <michael@paquier.xyz> — 2026-07-06T02:41:06Z

    On Tue, Jun 23, 2026 at 11:02:57AM -0500, Baji Shaik wrote:
> Thanks for the review.

This patch was still very noisy.  If running a bunch of reloads, we'd
still emit a WARNING all the time even if none of the bounds change.
This can be easily avoided: save the previous bound value before
ProcessConfigFile(), then recheck and proceed if one of the values has
changed.

Tweaked that and a few more things, then applied the result.

> I have kept the autovacuum message change as 0002 since Michael
> suggested unifying the format strings. If the consensus is to
> leave the existing autovacuum message alone, I'm happy to drop it.

This is useful on HEAD, so done that as 7905416eef9b.

Thanks,
--
Michael