On Mon, Jul 15, 2019 at 6:59 AM Thomas Munro <thomas.mu...@gmail.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
