Thread: pg_control contents

At Tom Lane's suggestion, I am adding a field to pg_control to hold the
compile-time configuration of timestamp and time storage.

I notice that the compile-time locale settings are registered in that
same structure. And that they depend on NAMEDATALEN, which is *not* in
that structure. istm that it should be, and I'll go ahead and add it
barring objections. Comments?
                        - Thomas

Thomas Lockhart <thomas@fourpalms.org> writes:
> I notice that the compile-time locale settings are registered in that
> same structure. And that they depend on NAMEDATALEN,

They do?  That would be fairly broken if so; sizeof(ControlFileData)
has to be independent of configurable settings, else you'll not get as
far as inspecting any of its contents (because the CRC check will fail
if computed over the wrong number of bytes).  But it looks to me like
LOCALE_NAME_BUFLEN is hardwired at 128.

> which is *not* in
> that structure. istm that it should be, and I'll go ahead and add it
> barring objections. Comments?

Putting NAMEDATALEN into the struct does seem like a good idea, and
perhaps FUNC_MAX_ARGS as well, since the system catalogs will be
unreadable if these numbers are wrong.  I think it's just an oversight
that we didn't put these values in pg_control to start with.

Don't forget to bump PG_CONTROL_VERSION.
        regards, tom lane

> > I notice that the compile-time locale settings are registered in that
> > same structure. And that they depend on NAMEDATALEN,
> They do?  That would be fairly broken if so; sizeof(ControlFileData)
> has to be independent of configurable settings, else you'll not get as
> far as inspecting any of its contents (because the CRC check will fail
> if computed over the wrong number of bytes).  But it looks to me like
> LOCALE_NAME_BUFLEN is hardwired at 128.

Ah. I should have looked before sending the mail; I was working on this
several days ago...

> Putting NAMEDATALEN into the struct does seem like a good idea, and
> perhaps FUNC_MAX_ARGS as well, since the system catalogs will be
> unreadable if these numbers are wrong.  I think it's just an oversight
> that we didn't put these values in pg_control to start with.

OK, I'll add NAMEDATALEN, FUNC_MAX_ARGS, and LOCALE_NAME_BUFLEN. Any
more?

> Don't forget to bump PG_CONTROL_VERSION.

I'd like to change this to the yyyymmddN format used in the catalog
version number (it is currently an integer set to ~71). It should make
it much easier to guess at code vintages from problem reports (if
nothing else), right?
                     - Thomas

Thomas Lockhart <thomas@fourpalms.org> writes:
> OK, I'll add NAMEDATALEN, FUNC_MAX_ARGS, and LOCALE_NAME_BUFLEN. Any
> more?

No, you're missing my point: there is zero value in adding
LOCALE_NAME_BUFLEN as an explicit field in ControlFileData.
The entire physical layout of ControlFileData has to be implicit in
PG_CONTROL_VERSION, because that is the only field we can reasonably
check before computing the CRC --- and if we don't have the correct
sizeof(ControlFileData), the CRC check will surely fail.  Therefore,
any change in LOCALE_NAME_BUFLEN would have to be signaled by bumping
PG_CONTROL_VERSION, *not* by any change in some other field inside
ControlFileData.  Look at the code that reads and validates pg_control
in xlog.c.

If there are other configurable parameters that can affect the format of
system catalogs, then by all means let's add 'em.  Nothing comes to mind
however.

>> Don't forget to bump PG_CONTROL_VERSION.

> I'd like to change this to the yyyymmddN format used in the catalog
> version number (it is currently an integer set to ~71). It should make
> it much easier to guess at code vintages from problem reports (if
> nothing else), right?

Actually, I deliberately did not use yyyymmdd for PG_CONTROL_VERSION,
because I wanted it to be absolutely not confusable with
CATALOG_VERSION_NO.  I took the then major version number as being
probably sufficient --- I do not foresee us revising pg_control's layout
very often, certainly less than once per release.

If you want to change it to yyyyN (eg, 20021 for this change) I won't
object.  But let's not use a convention that makes it look just like
CATALOG_VERSION_NO.  I think that'd be a recipe for confusion.
        regards, tom lane

> > OK, I'll add NAMEDATALEN, FUNC_MAX_ARGS, and LOCALE_NAME_BUFLEN. Any
> > more?
> No, you're missing my point: there is zero value in adding
> LOCALE_NAME_BUFLEN as an explicit field in ControlFileData.
> The entire physical layout of ControlFileData has to be implicit in
> PG_CONTROL_VERSION, because that is the only field we can reasonably
> check before computing the CRC --- and if we don't have the correct
> sizeof(ControlFileData), the CRC check will surely fail.  Therefore,
> any change in LOCALE_NAME_BUFLEN would have to be signaled by bumping
> PG_CONTROL_VERSION, *not* by any change in some other field inside
> ControlFileData.  Look at the code that reads and validates pg_control
> in xlog.c.

Got all that the first time. You are saying what *should* happen wrt
bumping version numbers when resizing those string fields, but that
isn't the point at all. *If* that doesn't happen, we should gracefully
give as much information as possible about the *nature* of the problem.
Just because the CRC fails doesn't mean that there are some clues inside
the file as to why, or what it would take to fix the problem.

If LOCALE_NAME_BUFLEN changes size between writing and reading the
control file, the CRC *could* still be calculated correctly. Currently
that is not the case, but that doesn't mean that we have an ideal
implementation at the moment.

> If there are other configurable parameters that can affect the format of
> system catalogs, then by all means let's add 'em.  Nothing comes to mind
> however.
> >> Don't forget to bump PG_CONTROL_VERSION.
> > I'd like to change this to the yyyymmddN format used in the catalog
> > version number (it is currently an integer set to ~71). It should make
> > it much easier to guess at code vintages from problem reports (if
> > nothing else), right?
> Actually, I deliberately did not use yyyymmdd for PG_CONTROL_VERSION,
> because I wanted it to be absolutely not confusable with
> CATALOG_VERSION_NO.  I took the then major version number as being
> probably sufficient --- I do not foresee us revising pg_control's layout
> very often, certainly less than once per release.
> If you want to change it to yyyyN (eg, 20021 for this change) I won't
> object.  But let's not use a convention that makes it look just like
> CATALOG_VERSION_NO.  I think that'd be a recipe for confusion.

I don't agree that detailed information in the same style as other
information is a guarantee of future trouble; in some circles consistant
approaches are used to avoid other possible trouble. I'm not much
interested in fighting Yet Another Issue for this case. Will revert to
the current scheme of incremental integer number but would be willing to
discuss this at some future date if it comes up again.
                  - Thomas

Thomas Lockhart <thomas@fourpalms.org> writes:
> If LOCALE_NAME_BUFLEN changes size between writing and reading the
> control file, the CRC *could* still be calculated correctly.

There might be some value to storing sizeof(ControlFileData) explicitly,
so that that CRC calculation could be made.  But I still see none in
storing LOCALE_NAME_BUFLEN explicitly.  There is *no* difference between
"I changed LOCALE_NAME_BUFLEN at random" and "I added or reordered
fields in the struct at random".  In either case the file has to be
treated as completely useless, because we don't really know what's in
there at what offset.  And making either sort of change without bumping
PG_CONTROL_VERSION is simply a mistake that we cannot afford to make.
There are plenty of places in PG where ill-considered hacking will have
undesirable consequences; pg_control is just one more.

The value of storing sizeof(ControlFileData) explicitly would not be
that we could hope to extract data safely, but only that we could
distinguish "corrupt data" from "good data in an incompatible format"
with marginally more reliability than now.  But both of these are and
must be failure cases, so it's really not that interesting to
distinguish between them.
        regards, tom lane