| From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
|---|---|
| To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
| Cc: | Wang Hao <whberet(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Should buffer of initialization fork have a BM_PERMANENT flag |
| Date: | 2017-03-13 19:46:51 |
| Message-ID: | CA+TgmoZYOzNzYE5iYXCCJ_xXMeN00fej=QS02D9P70eCbqJ8HA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Jan 25, 2017 at 7:14 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> (Adding Robert in CC.)
>
> On Thu, Jan 26, 2017 at 4:34 AM, Wang Hao <whberet(at)gmail(dot)com> wrote:
>> An unlogged table has an initialization fork. The initialization fork does
>> not have an BM_PERMANENT flag when get a buffer.
>> In checkpoint (not shutdown or end of recovery), it will not write to disk.
>> after a crash recovery, the page of initialization fork will not correctly,
>> then make the main fork not correctly too.
>
> For init forks the flush need absolutely to happen, so that's really
> not good. We ought to fix BufferAlloc() appropriately here.
I agree with that, but I propose the attached version instead. It
seems cleaner to have the entire test for setting BM_PERMANENT in one
place rather than splitting it up as you did.
I believe this sets a record for the longest-lived data corruption bug
in a commit made by me. Six years and change, woohoo. :-(
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
| Attachment | Content-Type | Size |
|---|---|---|
| unlogged-flush-fix-rmh.patch | application/octet-stream | 1.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Meskes | 2017-03-13 20:04:08 | pgsql: Add test case for two phase commit. Also by Masahiko Sawada. |
| Previous Message | Tom Lane | 2017-03-13 19:45:01 | Re: [COMMITTERS] pgsql: Add amcheck extension to contrib. |