From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Tomas Vondra <tomas(at)vondra(dot)me> |
Subject: | Re: Allow io_combine_limit up to 1MB |
Date: | 2025-04-25 14:26:09 |
Message-ID: | 1956847.1745591169@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Andres Freund <andres(at)anarazel(dot)de> writes:
> void
> assign_io_max_combine_limit(int newval, void *extra)
> {
> io_max_combine_limit = newval;
> io_combine_limit = Min(io_max_combine_limit, io_combine_limit_guc);
> }
> void
> assign_io_combine_limit(int newval, void *extra)
> {
> io_combine_limit_guc = newval;
> io_combine_limit = Min(io_max_combine_limit, io_combine_limit_guc);
> }
> So we end up with a larger io_combine_limit than
> io_max_combine_limit. Hilarity ensues.
There's another thing that's rather tin-eared about these assign
hooks: the hook is not supposed to be setting the GUC variable by
itself. guc.c will do that. It's relatively harmless for these
int-valued GUCs to be assigned twice, but I think it might be
problematic if this coding pattern were followed for a string GUC.
I suggest instead
void
assign_io_max_combine_limit(int newval, void *extra)
{
io_combine_limit = Min(newval, io_combine_limit_guc);
}
void
assign_io_combine_limit(int newval, void *extra)
{
io_combine_limit = Min(io_max_combine_limit, newval);
}
> Besides the obvious fix, I think we should also add
> Assert(len <= io_max_combine_limit);
+1
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Yurii Rashkovskii | 2025-04-25 14:57:00 | XACT_EVENT_COMMIT placement |
Previous Message | Andres Freund | 2025-04-25 14:15:55 | Re: Allow io_combine_limit up to 1MB |