Re: GUC values - recommended way to declare the C variables?

Peter Smith <smithpb2250(at)gmail(dot)com> writes:
> It seems confusing to me that for the above code the initial value is
> "hardwired" in multiple places. Specifically, it looks tempting to
> just change the variable declaration value, but IIUC that's going to
> achieve nothing because it will just be overwritten by the
> "boot-value" during the GUC mechanism start-up.

Well, if you try that you'll soon discover it doesn't work ;-)

IIRC, the primary argument for hand-initializing GUC variables is to
ensure that they have a sane value even before InitializeGUCOptions runs.
Obviously, that only matters for some subset of the GUCs that could be
consulted very early in startup ... but it's not perfectly clear just
which ones it matters for.

> a) Sometimes the static variable is assigned some (dummy?) value that
> is not the same as the boot value
> - See src/backend/utils/misc/guc_tables.c, max_replication_slots boot
> value is 10
> - See src/backend/replication/slot.c, int max_replication_slots = 0;

That seems pretty bogus. I think if we're not initializing a GUC to
the "correct" value then we should just leave it as not explicitly
initialized.

> c) Sometimes the GUC C variables don't even have a comment saying that
> they are GUC variables, so it is not at all obvious their initial
> values are going to get overwritten by some external mechanism.

That's flat out sloppy commenting. There are a lot of people around
here who seem to think comments are optional :-(

> SUGGESTION
> /*
> * GUC variables. Initial values are assigned at startup via
> InitializeGUCOptions.
> */
> int max_logical_replication_workers = 0;
> int max_sync_workers_per_subscription = 0;

1. Comment far wordier than necessary. In most places we just
annotate these as "GUC variables", and I think that's sufficient.
You're going to have a hard time getting people to write more
than that anyway.

2. I don't agree with explicitly initializing to a wrong value.
It'd be sufficient to do

int max_logical_replication_workers;
int max_sync_workers_per_subscription;

which would also make it clearer that initialization happens
through some other mechanism.

regards, tom lane