Re: Compression of bigger WAL records

Hi Andrey,

Thank you for the patch.

I like the idea of compacting the WAL, but I think there's a problem in the
current v1 design with not advancing the page LSN.

FlushBuffer() implements the WAL-before-data rule entirely through the page
LSN:

recptr = BufferGetLSN(buf);
...
if (pg_atomic_read_u64(&buf->state) & BM_PERMANENT)
XLogFlush(recptr);

/* "Force XLOG flush up to buffer's LSN. This implements the basic
WAL rule that log updates must hit disk before any of the
data-file changes they describe do." */

The mechanism today is that a page's permanent LSN is always at least the
LSN of some WAL record that references that block in the current
checkpoint cycle.
The v1 patch breaks this for hint-only changes. The compact record goes in
at LSN L, but the page LSN stays at whatever it was, let's call it L0, from
an
earlier checkpoint cycle, so XLogFlush(L0) is a no-op and the page can be
written while record L is still sitting in wal_buffers.

Because the page LSN is left alone, nothing ties record L to the page it
describes.
If the page is written first and the crash comes before L is flushed, the
hint bits end up present on the page while WAL holds no
record of them, and recovery has nothing to replay, so the page carries a
change that WAL never recorded.

So, I think the record needs the PageSetLSN().

Regards,
Surya Poondla