On 25/03/2019 15:21, Heikki Linnakangas wrote:
On 25/03/2019 09:57, David Steele wrote:
On 2/6/19 2:08 PM, Andrey Lepikhov wrote:
The patchset had a problem with all-zero pages, has appeared at index
build stage: the generic_log_relation() routine sends all pages into the
WAL. So  lsn field at all-zero page was initialized and the
PageIsVerified() routine detects it as a bad page.
The solution may be:
1. To improve index build algorithms and eliminate the possibility of
not used pages appearing.
2. To mark each page as 'dirty' right after initialization. In this case
we will got 'empty' page instead of the all-zeroed.
3. Do not write into the WAL all-zero pages.

Hmm. When do we create all-zero pages during index build? That seems pretty surprising.
GIST uses buffered pages. During GIST build it is possible (very rarely) what no one index tuple was written to the block page before new block was allocated. And the page has become an all-zero page. You can't have problems in the current GIST code, because it writes into the WAL only changed pages. But the idea of the patch is traversing blocks of index relation one-by-one after end of index building process and write this blocks to the WAL. In this case we will see all-zeroed pages and need to check it.

On 04.02.2019 10:04, Michael Paquier wrote:
On Tue, Dec 18, 2018 at 10:41:48AM +0500, Andrey Lepikhov wrote:
Ok. It is used only for demonstration.

The latest patch set needs a rebase, so moved to next CF, waiting on
author as this got no reviews.

The patch no longer applies so marked Waiting on Author.

Alexander, Heikki, are either of you planning to review the patch in
this CF?

I had another quick look.

I still think using the "generic xlog AM" for this is a wrong level of abstraction, and we should use the XLOG_FPI records for this directly. We can extend XLOG_FPI so that it can store multiple pages in a single record, if it doesn't already handle it.

Another counter-point to using the generic xlog record is that you're currently doing unnecessary two memcpy's of all pages in the index, in GenericXLogRegisterBuffer() and GenericXLogFinish(). That's not free.

I guess the generic_log_relation() function can stay where it is, but it should use XLogRegisterBuffer() and XLogInsert() directly.
Ok. This patch waited feedback for a long time. I will check the GIST code changes from previous review and will try to use your advice.

- Heikki

--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company

Reply via email to