On 01/31/2017 01:51 AM, Peter Geoghegan wrote:
* If you're writing out any old bit pattern, I suggest using 0x7F bytes rather than nul bytes (I do agree that it does seem worth making this some particular bit pattern, so as to not have to bother with Valgrind suppressions and so on). That pattern immediately gives you a hint on where to look for more information when there is a problem. To me, it suggests "this is something weird". We don't want this to happen very often.
Somehow writing zeros still feels more natural to me. That's what you'd get if you just seeked past the end of file, too, for example. I understand your point of using 0x7F to catch bugs, but an all-zeros page is a tell-tale too. So I went with all-zeros, anyway.
* I think that you should put the new code into a new function, called ltsAllocBlocksUntil(), or similar -- this can do the BufFileWrite() stuff itself, with a suitable custom defensive elog(ERROR) message. That way, the only new branch needed in ltsWriteBlock() is "if (blocknum > lts->nBlocksWritten)" (maybe use unlikely() too?), and you can make it clear that ltsAllocBlocksUntil() is a rarely needed thing, which seems appropriate.
I started to refactor this with something like ltsAllocBlocksUntil(), but in the end, it just seemed like more almost identical code with ltsWriteBlock.
We really don't want ltsAllocBlocksUntil() logic to be called very often, and certainly not to write more than 1 or 2 blocks at a time (no more than 1 with current usage). After all, that would mean writing to the same position twice or more, for no reason at all. Maybe note in comments that it's only called at the end of a run in practice, or something to that effect. Keeping writes sequential is very important, to keep logtape block reclamation effective.
Added a comment explaining that. Pushed with those little changes. Thanks! - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
