Thread: Reading recovery.conf earlier
I'm planning to read recovery.conf earlier in the startup process, so we can make a few things more "recovery aware". It's a nice-to-have only. This won't be part of the HS patch though. Proposal is to split out the couple of lines in readRecoveryCommandFile() that set important state and make it read in an option block that can be used by caller. It would then be called by both postmaster (earlier in startup) and again later by startup process, as happens now. I want to do it that way so I can read file before we create shared memory, so I don't have to worry about passing details via shared memory itself. It will allow some tidy up in HS patch but isn't going to be intrusive. Not thinking about lexers and stuff though at this stage, even if it is on the todo list. Any vetos? -- Simon Riggs www.2ndQuadrant.com
Simon Riggs <simon@2ndQuadrant.com> writes:
> I'm planning to read recovery.conf earlier in the startup process, so we
> can make a few things more "recovery aware". It's a nice-to-have only.
Say what? It's read at the beginning already.
regards, tom lane
On Sat, 2009-12-05 at 15:43 -0500, Tom Lane wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: > > I'm planning to read recovery.conf earlier in the startup process, so we > > can make a few things more "recovery aware". It's a nice-to-have only. > > Say what? It's read at the beginning already. Before the beginning then. :-) Two reasons to move it earlier in the startup sequence of the server * Some data structures are only required in HS mode. We don't know we're in HS mode until we created shared memory and started the Startup process. If we knew ahead of time, we could skip adding the structures. * Some things in postgresql.conf need to be overridden in HS mode, for example default_transaction_read_only = false. Again, we don't know we're in HS mode until later. So I would want to read recovery.conf before we read postgresql.conf Also, AFAIK, it's not easily possible to have dependencies between settings in postgresql.conf, unless the dependencies are expressed by ordering the dependent parameters in alphabetic order. So putting the parameters in postgresql.conf wouldn't help much. -- Simon Riggs www.2ndQuadrant.com
On Sun, Dec 6, 2009 at 2:49 AM, Simon Riggs <simon@2ndquadrant.com> wrote: > Proposal is to split out the couple of lines in > readRecoveryCommandFile() that set important state and make it read in > an option block that can be used by caller. It would then be called by > both postmaster (earlier in startup) and again later by startup process, > as happens now. I want to do it that way so I can read file before we > create shared memory, so I don't have to worry about passing details via > shared memory itself. I agree with the proposal that postmaster reads the recovery.conf. Because this would enable all child processes to easily obtain the parameter values in that, like GUC parameters. But I'm not sure why recovery.conf should be read separately by postmaster and the startup process. How about making postmaster read all of that for the simplification? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
On Mon, 2009-12-07 at 13:03 +0900, Fujii Masao wrote: > On Sun, Dec 6, 2009 at 2:49 AM, Simon Riggs <simon@2ndquadrant.com> wrote: > > Proposal is to split out the couple of lines in > > readRecoveryCommandFile() that set important state and make it read in > > an option block that can be used by caller. It would then be called by > > both postmaster (earlier in startup) and again later by startup process, > > as happens now. I want to do it that way so I can read file before we > > create shared memory, so I don't have to worry about passing details via > > shared memory itself. > > I agree with the proposal that postmaster reads the recovery.conf. > Because this would enable all child processes to easily obtain the > parameter values in that, like GUC parameters. OK > But I'm not sure why recovery.conf should be read separately by > postmaster and the startup process. How about making postmaster > read all of that for the simplification? That sounds better but is more complex. We'd have to read in the file, store in memory, then after shared memory is up put the data somewhere so the startup process can read it. (Remember Windows). What postgresql.conf already does is read file separately in each process, so no data passing. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs wrote: > What postgresql.conf already does is read file separately in each > process, so no data passing. No it doesn't. Postmaster reads the file once, and backends inherit the values at fork(). In EXEC_BACKEND case, postmaster writes all the non-default values to a separate file, which the child process reads at startup. Reading the file separately in each process would cause trouble with PGC_POSTMASTER params. All backends must agree on their values. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Mon, 2009-12-07 at 10:13 +0200, Heikki Linnakangas wrote: > Simon Riggs wrote: > > What postgresql.conf already does is read file separately in each > > process, so no data passing. > > No it doesn't. Postmaster reads the file once, and backends inherit the > values at fork(). In EXEC_BACKEND case, postmaster writes all the > non-default values to a separate file, which the child process reads at > startup. As I mentioned, the difficulty is always the Windows case. I wasn't aware we wrote a separate file in that case, so that does potentially effect my thinking. > Reading the file separately in each process would cause trouble with > PGC_POSTMASTER params. All backends must agree on their values. Looking at the parameters in recovery.conf I don't believe they would cause problems if read twice. So I think reading file twice would still be simplest way forwards. If you really think that changing the file is a possibility between processes reading them, then I would just take a full temp copy of the file, read it in postmaster, read it in startup, then delete temp file. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs wrote: > On Mon, 2009-12-07 at 10:13 +0200, Heikki Linnakangas wrote: >> Reading the file separately in each process would cause trouble with >> PGC_POSTMASTER params. All backends must agree on their values. > > Looking at the parameters in recovery.conf I don't believe they would > cause problems if read twice. So I think reading file twice would still > be simplest way forwards. Yeah, I don't know if that's an issue for recovery.conf, I was just pointing out that that's one reason it's done that way for postgresql.conf. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Mon, Dec 7, 2009 at 5:28 PM, Simon Riggs <simon@2ndquadrant.com> wrote: > If you really think that changing the file is a possibility between > processes reading them, then I would just take a full temp copy of the > file, read it in postmaster, read it in startup, then delete temp file. This seems more robust because processes which are started long after postmaster has started might use recovery.conf in the future (e.g., walreceiver in SR, read-only backends). Also, in Windows, writing only non-default values into a temp file like GUC is good for a process (like backend) which might be started many times. Reading the full of the file every startup of such process would somewhat harm the performance. Though, of course, this is overkill for your purpose. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
On Mon, 2009-12-07 at 19:26 +0900, Fujii Masao wrote: > On Mon, Dec 7, 2009 at 5:28 PM, Simon Riggs <simon@2ndquadrant.com> wrote: > > If you really think that changing the file is a possibility between > > processes reading them, then I would just take a full temp copy of the > > file, read it in postmaster, read it in startup, then delete temp file. > > This seems more robust because processes which are started long after > postmaster has started might use recovery.conf in the future (e.g., > walreceiver in SR, read-only backends). How does this sound? At startup we will delete recovery.conf.running, if it exists. At startup recovery.conf will be copied to recovery.conf.running, which will be the file read by any additional processes that read recovery.conf during this execution. The permissions on the file will then be set to read-only. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs wrote: > How does this sound? > > At startup we will delete recovery.conf.running, if it exists. > At startup recovery.conf will be copied to recovery.conf.running, which > will be the file read by any additional processes that read > recovery.conf during this execution. The permissions on the file will > then be set to read-only. Why not just follow the example of postresql.conf? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Mon, 2009-12-07 at 19:07 +0200, Heikki Linnakangas wrote: > Simon Riggs wrote: > > How does this sound? > > > > At startup we will delete recovery.conf.running, if it exists. > > At startup recovery.conf will be copied to recovery.conf.running, which > > will be the file read by any additional processes that read > > recovery.conf during this execution. The permissions on the file will > > then be set to read-only. > > Why not just follow the example of postresql.conf? Much better idea. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs <simon@2ndQuadrant.com> writes:
> On Mon, 2009-12-07 at 19:07 +0200, Heikki Linnakangas wrote:
>> Why not just follow the example of postresql.conf?
> Much better idea.
Rather than reinventing all the infrastructure associated with GUCs,
maybe we should just make the recovery parameters *be* GUCs. At least
for all the ones that could be of interest outside the recovery
subprocess itself.
As an example of the kind of thing you'll find yourself coding if you
make an independent facility: how will people find out the active
values?
regards, tom lane
On Mon, 2009-12-07 at 19:21 -0500, Tom Lane wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: > > On Mon, 2009-12-07 at 19:07 +0200, Heikki Linnakangas wrote: > >> Why not just follow the example of postresql.conf? > > > Much better idea. > > Rather than reinventing all the infrastructure associated with GUCs, > maybe we should just make the recovery parameters *be* GUCs. At least > for all the ones that could be of interest outside the recovery > subprocess itself. > > As an example of the kind of thing you'll find yourself coding if you > make an independent facility: how will people find out the active > values? You're right, I was literally just writing that code. Also, currently I have two parameters: wal_standby_info and recovery_connections. If this was a GUC, then I could just have one parameter: recovery_connections. So, much agreed. -- Simon Riggs www.2ndQuadrant.com