Re: Allow io_combine_limit up to 1MB

Andres Freund <andres@anarazel.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