Re: Adding SMGR discriminator to buffer tags

On Mon, Jul 15, 2019 at 6:59 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> That's an enum, so it works out to a word per record. The obvious way
> to avoid increasing the size is shove the SMGR ID into the same space
> that holds the forknum. Unlike BufferTag, where forknum currently
> swims in 32 bits which this patch chops in half, XLogRecorBlockHeader
> is already crammed into a uint8 fork_flags of which it has only the
> lower nibble, and the upper nibble is used for eg BKP_BLOCK_xxx flag
> bits, and there isn't even a spare bit to say 'has non-zero SMGR ID'.
> Rats. I suppose I could change it to a byte. I wonder if one extra
> byte per WAL record is acceptable. Anyone?

OK, I'll bite: I don't like it. I think this patch is more about how
people feel about things than it is about a technically necessary
change, and I'm absolutely OK with that up to the point where it
starts to inflict measurable costs on our users. Making WAL records
bigger in common cases, even by 1 byte, is a measurable cost. And
there are a few other minor costs too: we whack around a bunch of
internal APIs, and we force a pg_buffercache version bump. And I am
of the opinion that none of those costs, big or small, are buying us
anything technically. I am OK with being convinced otherwise, but
right now I am not convinced.

To set forth my argument: I think magic database OIDs are just fine.
The contrary arguments as I understand them are (1) stuff might break
if there's no matching entry in pg_database, or if there is, and (2)
some hypothetical smgr might need the database OID as a discriminator.
My counter-arguments are (1) we can fix that by writing the
appropriate code and it doesn't even seem very hard and (2) tough
noogies. To expand on (2) slightly, the proposals on the table do not
need that, the existing smgr does not need that, and there's no reason
to suppose that future proposals would require that either, because
2^32 relfilenodes of up to 2^32 blocks each is a lot, and you
shouldn't need another 2^32 bits. If someone does come up with a
proposal that needs those bits, perhaps because it lives within a
database rather than being a global object like SLRU or undo data,
maybe it should be a new kind of AM rather than a new smgr. And if
not, then maybe we should leave it to that hypothetical patch to solve
that hypothetical problem, because right now we're just speculating
that another 32 bits will fix it, which we can't really know, because
if we're hypothesizing the existence of a patch that needs more bits,
we could also hypothesize that it needs more than 32 of them.

If we absolutely have to keep driving down this course, you could
probably steal a bit from the fork number nibble to indicate a
non-default smgr. Even if there are only 2 bits there, you could use
1 for non-default smgr and 1 for non-default fork number, and then in
the common case of references to the default block of the default
smgr, you wouldn't be spending anything additional ... assuming you
don't count the CPU cycles to encode and decode a more complex WAL
record format.

But how about just using a magic database OID?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company