Re: Should buffer of initialization fork have a BM_PERMANENT flag

Hello,

I wanted to review the patch. But the patch is applied with errors. I've
rebased the local copy and have done review on it. I'm not sure is it
properly to send rebased patch by reviewer, so I haven't sent it to
avoid confuses.

On 29.01.2017 17:00, Michael Paquier wrote:
> Attached is what I have in mind for HEAD. btree, gist, spgist and
> bloom indexes are changed so as the init forks created go through the
> shared buffers instead of having their empty() routines handle the
> flush of the page created. This removes any kind of race conditions
> between the checkpointer and the init fork creations, which is I think
> a good thing.

I think this is good fixes. I've checked them. And in my opinion they
are correct.

The code also is good.

>
> Here are the tests I have done.
> First running those commands to create all types of indexes.
> create extension bloom;
> create extension btree_gist;
> create extension btree_gin;
> create unlogged table foo (a int);
> create index foo_bt on foo(a);
> create index foo_bloom on foo using bloom(a);
> create index foo_gin on foo using gin (a);
> create index foo_gist on foo using gist (a);
> create index foo_brin on foo using brin (a);
> create unlogged table foo_geo (a box);
> create index foo_spgist ON foo_geo using spgist(a);
> checkpoint;
>
> Then crash the server, restart it, and the following vacuums are able
> to complete.
> vacuum foo;
> vacuum foo_geo;
>

I've done this tests. Before the patch server crashes on vacuum command.
After applying the patch server doesn't crash on vacuum command.

I have run regression and TAP tests. They all passed without error.

I think the patch can be marked as "Ready for Committer" after rebase.

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company