On Tue, Mar 20, 2018 at 12:00:47PM +0530, Dilip Kumar wrote:
> Yeah, you are right. Fixed.
So I have been spending a couple of hours playing with your patch, and
tested various configurations manually, like switch the fpw switch to on
and off while running in parallel pgbench. I have also tested
promotions, etc.
While the patch does its job, it is possible to simplify a bit more the
calls to InitXLogInsert(). Particularly, the one in CreateCheckpoint()
is basically useless for the checkpointer, still it is useful for the
startup process when trigerring an end-in-recovery checkpoint. So one
additional cleanup would be to move the call in CreateCheckpoint() to
bootstrap.c for the startup process. In order to test that, please make
sure to create fallback_promote at the root of the data folder before
sending SIGUSR2 to the startup process which would trigger the pre-9.3
promotion where the end-of-recovery checkpoint is triggered by the
startup process itself.
+ /* Initialize the working areas for constructing WAL records. */
+ InitXLogInsert();
Instead of having the same comment for each process calling
InitXLogInsert() multiple times, I think that it would be better to
complete the comment in bootstrap.c where is written "XLOG operations".
Here is a suggestion:
/*
* Initialize WAL-related operations and enter the main loop of each
* process. InitXLogInsert is called for each process which can
* generate WAL. While this is wasteful for processes started on
* a standby, this gives the guarantee that initialization of WAL
* insertion areas is able to happen in a consistent way and out of
* any critical sections so as the facility is usable when a promotion
* is triggered.
*/
What do you think?
--
Michael