On Fri, Mar 23, 2018 at 1:19 PM, Michael Paquier <michael@paquier.xyz> wrote:
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 StarupXLOG, just before the CreateCheckPoint() call, we are calling LocalSetXLogInsertAllowed(). So, I am thinking can we just remove InitXLogInsert from CreateCheckpoint. And, we don't need to move it to bootstrap.c. Or am I missing something?
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. */
