Prev: [HACKERS] missing data in information_schema grant_* tables?
Next: [HACKERS] ECPG documentation patch
From: Heikki Linnakangas on 15 Jan 2010 06:28
Fujii Masao wrote:
> On Wed, Dec 9, 2009 at 6:25 PM, Fujii Masao <masao.fujii(a)gmail.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
From: Heikki Linnakangas on 15 Jan 2010 11:42
Greg Stark wrote:
> What if someone takes a hot backup while an unlogged operation is in progress.

Can't do that, pg_start_backup() throws an error if archive_mode=off.

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Fujii Masao on 16 Jan 2010 01:16
On Fri, Jan 15, 2010 at 8:28 PM, Heikki Linnakangas
<heikki.linnakangas(a)enterprisedb.com> wrote:
> 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?

Thanks a lot! Your change seems to be OK.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Fujii Masao on 18 Jan 2010 00:08
On Sat, Jan 16, 2010 at 3:16 PM, Fujii Masao <masao.fujii(a)gmail.com> wrote:
>> Attached is an updated patch, doing the above. Am I missing anything?
>
> Thanks a lot! Your change seems to be OK.

We'll need to do some more work after the following patch
has been committed.
http://archives.postgresql.org/pgsql-hackers/2010-01/msg01715.php

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on 18 Jan 2010 03:56
On Fri, 2010-01-15 at 13:28 +0200, Heikki Linnakangas wrote:
> 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().

Can we call that XLogReportUnloggedStatement() or similar?
XlogSkipLogging() sounds like a request rather than a mark/report/record
type of action.

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

Sounds OK and works with Hot Standby.

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

 |  Next  |  Last
Pages: 1 2
Prev: [HACKERS] missing data in information_schema grant_* tables?
Next: [HACKERS] ECPG documentation patch