Horton

Re: [HACKERS] WAL logging problem in 9.4.3?

Andrew Dunstan <andrew.dunstan@2ndquadrant.com>

View thread View on postgr.es
From: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>, Robert Haas <robertmhaas@gmail.com>, Michael Paquier <michael@paquier.xyz>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2018-07-27T19:26:24Z
Lists: pgsql-hackers 

On 07/18/2018 10:58 AM, Heikki Linnakangas wrote:
> On 18/07/18 16:29, Robert Haas wrote:
>> On Wed, Jul 18, 2018 at 9:06 AM, Michael Paquier 
>> <michael@paquier.xyz> wrote:
>>>> What's wrong with the approach proposed in
>>>> http://postgr.es/m/55AFC302.1060805@iki.fi ?
>>>
>>> For back-branches that's very invasive so that seems risky to me
>>> particularly seeing the low number of complaints on the matter.
>>
>> Hmm. I think that if you disable the optimization, you're betting that
>> people won't mind losing performance in this case in a maintenance
>> release.  If you back-patch Heikki's approach, you're betting that the
>> committed version doesn't have any bugs that are worse than the status
>> quo.  Personally, I'd rather take the latter bet.  Maybe the patch
>> isn't all there yet, but that seems like something we can work
>> towards.  If we just give up and disable the optimization, we won't
>> know how many people we ticked off or how badly until after we've done
>> it.
>
> Yeah. I'm not happy about backpatching a big patch like what I 
> proposed, and Kyotaro developed further. But I think it's the least 
> bad option we have, the other options discussed seem even worse.
>
> One way to review the patch is to look at what it changes, when 
> wal_level is *not* set to minimal, i.e. what risk or overhead does it 
> pose to users who are not affected by this bug? It seems pretty safe 
> to me.
>
> The other aspect is, how confident are we that this actually fixes the 
> bug, with least impact to users using wal_level='minimal'? I think 
> it's the best shot we have so far. All the other proposals either 
> don't fully fix the bug, or hurt performance in some legit cases.
>
> I'd suggest that we continue based on the patch that Kyotaro posted at 
> https://www.postgresql.org/message-id/20180330.100646.86008470.horiguchi.kyotaro%40lab.ntt.co.jp.
>



I have just spent some time reviewing Kyatoro's patch. I'm a bit 
nervous, too, given the size. But I'm also nervous about leaving things 
as they are. I suspect the reason we haven't heard more about this is 
that these days use of "wal_level = minimal" is relatively rare.

I like the fact that this is closer to being a real fix rather than just 
throwing out the optimization. Like Heikki I've come round to the view 
that something like this is the least bad option.

The code looks good to me - some comments might be helpful in 
heap_xlog_update()

Do we want to try this on HEAD and then backpatch it? Do we want to add 
some testing along the lines Michael suggested?

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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