Re: [HACKERS] New XLOG record indicating WAL-skipping

2010-01-18 Thread Simon Riggs
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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] New XLOG record indicating WAL-skipping

2010-01-18 Thread Heikki Linnakangas
Simon Riggs wrote:
 Can we call that XLogReportUnloggedStatement() or similar?
 XlogSkipLogging() sounds like a request rather than a mark/report/record
 type of action.

Agreed. I vote for XLogReportUnloggedOperation(). I'll change it to that
before committing, unless Fujii beats me to it.

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

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


Re: [HACKERS] New XLOG record indicating WAL-skipping

2010-01-18 Thread Fujii Masao
On Mon, Jan 18, 2010 at 9:17 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Agreed. I vote for XLogReportUnloggedOperation().

OK.

 I'll change it to that
 before committing, unless Fujii beats me to it.

Yeah, please go ahead.

Regards,

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

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


Re: [HACKERS] New XLOG record indicating WAL-skipping

2010-01-17 Thread Fujii Masao
On Sat, Jan 16, 2010 at 3:16 PM, Fujii Masao masao.fu...@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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] New XLOG record indicating WAL-skipping

2010-01-15 Thread Heikki Linnakangas
Fujii Masao wrote:
 On Wed, Dec 9, 2009 at 6:25 PM, Fujii Masao masao.fu...@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
? GNUmakefile
? b
? config.log
? config.status
? config.status.lineno
? configure.lineno
? gin-splay-1.patch
? gin-splay-2.patch
? gin-splay-3.patch
? md-1.c
? md-1.patch
? temp-file-resowner-2.patch
? contrib/pgbench/fsynctest
? contrib/pgbench/fsynctest.c
? contrib/pgbench/fsynctestfile
? contrib/spi/.deps
? doc/src/sgml/HTML.index
? doc/src/sgml/bookindex.sgml
? doc/src/sgml/features-supported.sgml
? doc/src/sgml/features-unsupported.sgml
? doc/src/sgml/version.sgml
? src/Makefile.global
? src/backend/aaa.patch
? src/backend/postgres
? src/backend/access/common/.deps
? src/backend/access/gin/.deps
? src/backend/access/gist/.deps
? src/backend/access/hash/.deps
? src/backend/access/heap/.deps
? src/backend/access/index/.deps
? src/backend/access/nbtree/.deps
? src/backend/access/transam/.deps
? src/backend/bootstrap/.deps
? src/backend/catalog/.deps
? src/backend/commands/.deps
? src/backend/executor/.deps
? src/backend/foreign/.deps
? src/backend/foreign/dummy/.deps
? src/backend/foreign/postgresql/.deps
? src/backend/lib/.deps
? src/backend/libpq/.deps
? src/backend/main/.deps
? src/backend/nodes/.deps
? src/backend/optimizer/geqo/.deps
? src/backend/optimizer/path/.deps
? src/backend/optimizer/plan/.deps
? src/backend/optimizer/prep/.deps
? src/backend/optimizer/util/.deps
? src/backend/parser/.deps
? src/backend/po/af.mo
? src/backend/po/cs.mo
? src/backend/po/de.mo
? src/backend/po/es.mo
? src/backend/po/fr.mo
? src/backend/po/hr.mo
? src/backend/po/hu.mo
? src/backend/po/it.mo
? src/backend/po/ja.mo
? src/backend/po/ko.mo
? src/backend/po/nb.mo
? src/backend/po/nl.mo
? src/backend/po/pl.mo
? src/backend/po/pt_BR.mo
? src/backend/po/ro.mo
? src/backend/po/ru.mo
? src/backend/po/sk.mo
? src/backend/po/sl.mo
? src/backend/po/sv.mo
? src/backend/po/tr.mo
? src/backend/po/zh_CN.mo
? src/backend/po/zh_TW.mo
? src/backend/port/.deps
? src/backend/postmaster/.deps
? src/backend/regex/.deps
? src/backend/replication/.deps
? src/backend/replication/walreceiver/.deps
? src/backend/rewrite/.deps
? src/backend/snowball/.deps
? src/backend/snowball/snowball_create.sql
? src/backend/storage/buffer/.deps
? src/backend/storage/file/.deps
? src/backend/storage/freespace/.deps
? src/backend/storage/ipc/.deps
? src/backend/storage/large_object/.deps
? src/backend/storage/lmgr/.deps
? src/backend/storage/page/.deps
? src/backend/storage/smgr/.deps
? src/backend/tcop/.deps
? src/backend/tsearch/.deps
? src/backend/utils/.deps
? src/backend/utils/probes.h
? src/backend/utils/adt/.deps
? src/backend/utils/cache/.deps
? src/backend/utils/error/.deps
? src/backend/utils/fmgr/.deps
? src/backend/utils/hash/.deps
? src/backend/utils/init/.deps
? src/backend/utils/mb/.deps
? src/backend/utils/mb/Unicode/BIG5.TXT
? src/backend/utils/mb/Unicode/CP950.TXT
? src/backend/utils/mb/conversion_procs/conversion_create.sql
? src/backend/utils/mb/conversion_procs/ascii_and_mic/.deps
? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/.deps
? src/backend/utils/mb/conversion_procs/euc2004_sjis2004/.deps
? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/.deps
? 

Re: [HACKERS] New XLOG record indicating WAL-skipping

2010-01-15 Thread Greg Stark
On Fri, Jan 15, 2010 at 11:28 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 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?

Hm, perhaps the timing is actually important? What if someone takes a
hot backup while an unlogged operation is in progress. The checkpoint
can occur and finish and the backup finish all while the unlogged
operation is happening. Then the replica can start restoring archived
logs from that point forward. In the original coding it sounds like
the replica would never notice the unlogged operation which might not
have been synced before the start of the initial hot backup.  If the
record occurs when the sync begins then the replica would be in
trouble if the checkpoint begins before the operation completed but
finished after the sync began and the record was emitted.

It seems like it's important that the record occur only after the sync
*completes* to be sure that if the replica doesn't see the record then
it knows the sync was done before its initial backup image was taken.

-- 
greg

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


Re: [HACKERS] New XLOG record indicating WAL-skipping

2010-01-15 Thread Heikki Linnakangas
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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers