Обсуждение: Re: Catalog version access
On Wed, Mar 3, 2021 at 8:16 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Vik Fearing <vik@postgresfriends.org> writes: > > On 3/3/21 6:35 PM, Peter Eisentraut wrote: > >> If what you want to know is whether a given binary can run against a > >> given data directory then CATALOG_VERSION_NO isn't the only thing you > >> need to check. The full truth of this is in ReadControlFile(). The > >> best way to get that answer is to start a server and see if it > >> complains. You can even grep the log for "It looks like you need to > >> initdb.". > > > In that case, what would everyone think about a `pg_ctl check` option? > > The trouble with Peter's recipe is that it doesn't work if there is > already a server instance running there (or at least I think we'll > bitch about the existing postmaster first, maybe I'm wrong). Now, > that's not such a big problem for the use-case you were describing. > But I bet if we expose this method as an apparently-general-purpose > pg_ctl option, there'll be complaints. Another option could be to provide a switch to the postmaster binary. Using pg_config as originally suggested is risky because you might pick up the wrong postmaster, but if you put it on the actual postmaster binary you certainly know which one you're on... As this is something that's primarily of interest to developers, it's also a bit lower weight than having a "heavy" solution like an entire mode for pg_ctl. -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
On 4/8/21, 2:58 AM, "Magnus Hagander" <magnus@hagander.net> wrote: > Another option could be to provide a switch to the postmaster binary. > Using pg_config as originally suggested is risky because you might > pick up the wrong postmaster, but if you put it on the actual > postmaster binary you certainly know which one you're on... As this is > something that's primarily of interest to developers, it's also a bit > lower weight than having a "heavy" solution like an entire mode for > pg_ctl. I was looking at the --check switch for the postgres binary recently [0], and this sounds like something that might fit in nicely there. In the attached patch, --check will also check the control file if one exists. Nathan [0] https://postgr.es/m/0545F7B3-70C0-4DE8-8C85-EAFE6113B7EE%40amazon.com
Вложения
On Mon, Aug 16, 2021 at 06:12:54PM +0000, Bossart, Nathan wrote: > I was looking at the --check switch for the postgres binary recently > [0], and this sounds like something that might fit in nicely there. > In the attached patch, --check will also check the control file if one > exists. This would not work on a running postmaster as CreateDataDirLockFile() is called beforehand, but we want this capability, no? Abusing a bootstrap option for this purpose does not look like a good idea, to be honest, especially for something only used internally now and undocumented, but we want to use something aimed at an external use with some documentation. Using a separate switch would be more adapted IMO. Also, I think that we should be careful with the read of the control file to avoid false positives? We can rely on an atomic read/write thanks to its maximum size of 512 bytes, but this looks like a lot what has been done recently with postgres -C for runtime GUCs, that *require* a read of the control file before grabbing the values we are interested in. -- Michael
Вложения
Thanks for taking a look! On 1/23/22, 7:31 PM, "Michael Paquier" <michael@paquier.xyz> wrote: > On Mon, Aug 16, 2021 at 06:12:54PM +0000, Bossart, Nathan wrote: >> I was looking at the --check switch for the postgres binary recently >> [0], and this sounds like something that might fit in nicely there. >> In the attached patch, --check will also check the control file if one >> exists. > > This would not work on a running postmaster as CreateDataDirLockFile() > is called beforehand, but we want this capability, no? I was not under the impression this was a requirement, based on the use-case discussed upthread [0]. > Abusing a bootstrap option for this purpose does not look like a good > idea, to be honest, especially for something only used internally now > and undocumented, but we want to use something aimed at an external > use with some documentation. Using a separate switch would be more > adapted IMO. This is the opposite of what Magnus proposed earlier [1]. Do we need a new pg_ctl option for this? It seems like it is really only intended for use by PostgreSQL developers, but perhaps there are other use-cases I am not thinking of. In any case, the pg_ctl option would probably end up using --check (or another similar mode) behind the scenes. > Also, I think that we should be careful with the read of > the control file to avoid false positives? We can rely on an atomic > read/write thanks to its maximum size of 512 bytes, but this looks > like a lot what has been done recently with postgres -C for runtime > GUCs, that *require* a read of the control file before grabbing the > values we are interested in. Sorry, I'm not following this one. In my proposed patch, the control file (if one exists) is read after CreateDataDirLockFile(), just like PostmasterMain(). Nathan [0] https://postgr.es/m/20210222022407.ecaygvx2ise6uwyz%40alap3.anarazel.de [1] https://postgr.es/m/CABUevEySovaEDci7c0DXOrV6c7JzWqYzfVwOiRUJxMao%3D9seEw%40mail.gmail.com
On Mon, Jan 24, 2022 at 08:40:08PM +0000, Bossart, Nathan wrote: > On 1/23/22, 7:31 PM, "Michael Paquier" <michael@paquier.xyz> wrote: >> On Mon, Aug 16, 2021 at 06:12:54PM +0000, Bossart, Nathan wrote: >>> I was looking at the --check switch for the postgres binary recently >>> [0], and this sounds like something that might fit in nicely there. >>> In the attached patch, --check will also check the control file if one >>> exists. >> >> This would not work on a running postmaster as CreateDataDirLockFile() >> is called beforehand, but we want this capability, no? > > I was not under the impression this was a requirement, based on the > use-case discussed upthread [0]. Hmm. I got a different impression as of this one: https://www.postgresql.org/message-id/3496407.1613955241@sss.pgh.pa.us But I can see downthread that this is not the case. Sorry for the confusion. >> Abusing a bootstrap option for this purpose does not look like a good >> idea, to be honest, especially for something only used internally now >> and undocumented, but we want to use something aimed at an external >> use with some documentation. Using a separate switch would be more >> adapted IMO. > > This is the opposite of what Magnus proposed earlier [1]. Do we need > a new pg_ctl option for this? It seems like it is really only > intended for use by PostgreSQL developers, but perhaps there are other > use-cases I am not thinking of. In any case, the pg_ctl option would > probably end up using --check (or another similar mode) behind the > scenes. Based on the latest state of the thread, I am understanding that we don't want a new option for pg_ctl for this feature, and using a bootstrap's --check for this purpose is not a good idea IMO. What I guess from Magnus' suggestion would be to add a completely different switch. Once you remove the requirement of a running server, we have basically what has been recently implemented with postgres -C for runtime-computed GUCs, because we already go through a read of the control file to be able to print those GUCs with their correct values. This also means that it is already possible to check if a data folder is compatible with a set of binaries with this facility, as any postgres -C command with a runtime GUC would trigger this check. Using any of the existing runtime GUCs may be confusing, but that would work. And I am not really convinced that we have any need to add a specific GUC for this purpose, be it a sort of is_controlfile_valid or controlfile_checksum (CRC32 of the control file). >> Also, I think that we should be careful with the read of >> the control file to avoid false positives? We can rely on an atomic >> read/write thanks to its maximum size of 512 bytes, but this looks >> like a lot what has been done recently with postgres -C for runtime >> GUCs, that *require* a read of the control file before grabbing the >> values we are interested in. > > Sorry, I'm not following this one. In my proposed patch, the control > file (if one exists) is read after CreateDataDirLockFile(), just like > PostmasterMain(). While looking at the patch, I was thinking about the fact that we may want to support the case even if a server is running. If we don't, my worries about the concurrent control file activities are moot. -- Michael
Вложения
On Tue, Jan 25, 2022 at 01:12:32PM +0900, Michael Paquier wrote: > Once you remove the requirement of a running server, we have basically > what has been recently implemented with postgres -C for > runtime-computed GUCs, because we already go through a read of the > control file to be able to print those GUCs with their correct > values. This also means that it is already possible to check if a > data folder is compatible with a set of binaries with this facility, > as any postgres -C command with a runtime GUC would trigger this > check. Using any of the existing runtime GUCs may be confusing, but > that would work. And I am not really convinced that we have any need > to add a specific GUC for this purpose, be it a sort of > is_controlfile_valid or controlfile_checksum (CRC32 of the control > file). Thinking more about this one, we can already do that, so I have marked the patch as RwF. Perhaps we could just add a GUC, but that feels a bit dummy. -- Michael
Вложения
On Mon, Jan 31, 2022 at 04:57:13PM +0900, Michael Paquier wrote: > On Tue, Jan 25, 2022 at 01:12:32PM +0900, Michael Paquier wrote: >> Once you remove the requirement of a running server, we have basically >> what has been recently implemented with postgres -C for >> runtime-computed GUCs, because we already go through a read of the >> control file to be able to print those GUCs with their correct >> values. This also means that it is already possible to check if a >> data folder is compatible with a set of binaries with this facility, >> as any postgres -C command with a runtime GUC would trigger this >> check. Using any of the existing runtime GUCs may be confusing, but >> that would work. And I am not really convinced that we have any need >> to add a specific GUC for this purpose, be it a sort of >> is_controlfile_valid or controlfile_checksum (CRC32 of the control >> file). > > Thinking more about this one, we can already do that, so I have > marked the patch as RwF. Perhaps we could just add a GUC, but that > feels a bit dummy. Sorry, I missed this thread earlier. You're right, we can just do something like the following to achieve basically the same result: postgres -D . -C data_checksums Unless Vik has any objections, this can probably be marked as Withdrawn. Perhaps we can look into providing a new option for "postgres" at some point in the future, but I don't sense a ton of demand at the moment. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com