Re: New XLOG record indicating WAL-skipping

Fujii Masao wrote:
> On Wed, Dec 9, 2009 at 6:25 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> Here is the patch:
>>
>> - Write an XLOG UNLOGGED record in WAL if WAL-logging is skipped for only
>> the reason that WAL archiving is not enabled and such record has not been
>> written yet.
>>
>> - Cause archive recovery to end if an XLOG UNLOGGED record is found during
>> it.
>
> Here's an updated version of my "New XLOG record indicating WAL-skipping" patch.
> http://archives.postgresql.org/pgsql-hackers/2009-12/msg00788.php

Thanks!

I don't like special-casing UNLOGGED records in XLogInsert and
ReadRecord(). Those functions are complicated enough already. The
special handling from XLogInsert() (and a few other places) is only
required because the UNLOGGED records carry no payload. That's easy to
avoid, just add some payload to them, doesn't matter what it is. And I
don't think ReadRecord() is the right place to emit the errors/warnings,
that belongs naturally in xlog_redo().

It might be useful to add some information in the records telling why
WAL-logging was skipped. It might turn out to be useful in debugging.
That also conveniently adds payload to the records, to avoid the
special-casing in XLogInsert() :-).

I think it's a premature optimization to skip writing the records if
we've written in the same session already. Especially with the 'reason'
information added to the records, it's nice to have a record of each
such operation. All operations that skip WAL-logging are heavy enough
that an additional WAL record will make no difference. I can see that it
was required to avoid the flooding from heap_insert(), but we can move
the XLogSkipLogging() call from heap_insert() to heap_sync().

Attached is an updated patch, doing the above. Am I missing anything?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com