Horton

Re: WAL logging problem in 9.4.3?

Michael Paquier <michael.paquier@gmail.com>

View thread View on postgr.es
From: Michael Paquier <michael.paquier@gmail.com>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@2ndquadrant.com>, david@pgmasters.net, Heikki Linnakangas <hlinnaka@iki.fi>, Simon Riggs <simon@2ndquadrant.com>, Andres Freund <andres@anarazel.de>, Fujii Masao <masao.fujii@gmail.com>, kleptog@svana.org, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Date: 2017-04-13T04:52:40Z
Lists: pgsql-hackers 
On Tue, Apr 11, 2017 at 5:38 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> Sorry, what I have just sent was broken.

You can use PROVE_TESTS when running make check to select a subset of
tests you want to run. I use that all the time when working on patches
dedicated to certain code paths.

>> - Relation has new members no_pending_sync and pending_sync that
>>   works as instant cache of an entry in pendingSync hash.
>> - Commit-time synchronizing is restored as Michael's patch.
>> - If relfilenode is replaced, pending_sync for the old node is
>>   removed. Anyway this is ignored on abort and meaningless on
>>   commit.
>> - TAP test is renamed to 012 since some new files have been added.
>>
>> Accessing pending sync hash occurred on every calling of
>> HeapNeedsWAL() (per insertion/update/freeze of a tuple) if any of
>> accessing relations has pending sync.  Almost of them are
>> eliminated as the result.

Did you actually test this patch? One of the logs added makes the
tests a long time to run:
2017-04-13 12:11:27.065 JST [85441] t/102_vacuumdb_stages.pl
STATEMENT:  ANALYZE;
2017-04-13 12:12:25.766 JST [85492] LOG:  BufferNeedsWAL: pendingSyncs
= 0x0, no_pending_sync = 0

-       lsn = XLogInsert(RM_SMGR_ID,
-                        XLOG_SMGR_TRUNCATE | XLR_SPECIAL_REL_UPDATE);
+           rel->no_pending_sync= false;
+           rel->pending_sync = pending;
+       }
It seems to me that those flags and the pending_sync data should be
kept in the context of backend process and not be part of the Relation
data...

+void
+RecordPendingSync(Relation rel)
I don't think that I agree that this should be part of relcache.c. The
syncs are tracked should be tracked out of the relation context.

Seeing how invasive this change is, I would also advocate for this
patch as only being a HEAD-only change, not many people are
complaining about this optimization of TRUNCATE missing when wal_level
= minimal, and this needs a very careful review.

Should I code something? Or Horiguchi-san, would you take care of it?
The previous crash I saw has been taken care of, but it's been really
some time since I looked at this patch...
-- 
Michael

Commits

  1. Add perl2host call missing from a new test file.

  2. Skip WAL for new relfilenodes, under wal_level=minimal.

  3. Revert "Skip WAL for new relfilenodes, under wal_level=minimal."

  4. Back-patch log_newpage_range().

  5. During heap rebuild, lock any TOAST index until end of transaction.

  6. In log_newpage_range(), heed forkNum and page_std arguments.

  7. Back-patch src/test/recovery and PostgresNode from 9.6 to 9.5.

  8. Reduce pg_ctl's reaction time when waiting for postmaster start/stop.

  9. Accelerate end-of-transaction dropping of relations

  10. Redesign the planner's handling of index-descent cost estimation.

  11. Make TRUNCATE do truncate-in-place when processing a relation that was created