GiST index build missing smgrimmedsync()?

I brought this up in [1] but wanted to start a dedicated thread.

Since 16fa9b2b30a357 GiST indexes are not built in shared buffers.
However, smgrimmedsync() is not done anywhere and skipFsync=true is
always passed to smgrwrite() and smgrextend(). So, if a checkpoint
starts and finishes after WAL-logging some of the index build pages and
there is a crash sometime before the dirty pages make it to permanent
storage, won't that data be lost?

(Also, as Heikki has pointed out before, passing skipFsync=false isn't
sufficient either if checkpoint finishes before
register_dirty_segment(), so seems like smgrimmesync() is needed.)

Seems like the following would address this:

diff --git a/src/backend/access/gist/gistbuild.c
b/src/backend/access/gist/gistbuild.c
index 9854116fca..364a7455ea 100644
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -454,9 +454,11 @@ gist_indexsortbuild(GISTBuildState *state)
smgrwrite(RelationGetSmgr(state->indexrel), MAIN_FORKNUM,
GIST_ROOT_BLKNO,
pagestate->page, true);
if (RelationNeedsWAL(state->indexrel))
+ {
log_newpage(&state->indexrel->rd_node, MAIN_FORKNUM,
GIST_ROOT_BLKNO,
pagestate->page, true);
-
+ smgrimmedsync(RelationGetSmgr(state->indexrel), MAIN_FORKNUM);
+ }
pfree(pagestate->page);
pfree(pagestate);
}

- Melanie

[1] https://www.postgresql.org/message-id/CAAKRu_ZmkCNz-%3D5U5wZyyi3%3DUsfs1btsTKc_L9r1ceFvbxE3kg%40mail.gmail.com