Обсуждение: Re: [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
Re: [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
От
Heikki Linnakangas
Дата:
On 31.01.2013 21:33, Simon Riggs wrote: > If anyone really wants me to revert, pls start new hackers thread to > discuss, or comment on changes. Yes, I still think this needs fixing or reverting. Let me reiterate my my complaints: 1. I don't like the check in ReadCheckPointRecord() that the WAL containing last and previous checkpoint still exists. Several reasons for that: 1.1. I don't think such a check is necessary to begin with. We replayed that WAL record a while ago, so there's no reason to believe that it's gone now. If there is a bug that causes that to happen, you're screwed with or without this patch. 1.2. If we do that check, and it fails because the latest checkpoint is not present, there should be a big fat warning in the log because something's wrong. If you ask for fast promotion, and the system doesn't do that, a log message is the least we can do. 1.3. Why check for the "prev" checkpoint? The latest checkpoint is enough to recover, so why insist that also the previous one is present, too? There are normal scenarios where it won't be, like just after recovering from a base backup. I consider it a bug that fast promotion doesn't work right after restoring from a base backup. 2. I don't like demoting the trigger file method to a second class citizen. I think we should make all functionality available through both methods. If there was a good reason for deprecating the trigger file method, I could live with that, but this patch is not such a reason. 3. I don't like conflating the promotion modes and shutdown modes in the pg_ctl option. Shutdown modes and promotion modes are separate concepts. The "fast" option is pretty clear, but why does "smart" mean "create an immediate checkpoint before promotion"? How is that smarter than the fast mode? The "pg_ctl --help" on that is a bit confusing too: > Options for stop, restart or promote: -m, --mode=MODE MODE can > be "smart", "fast", or "immediate" The "immediate" mode is not actually valid for "pg_ctl promote". That is clarified later in the output by listing out what the modes mean, but that above line is misleading, 4. I think fast promotion should be the default. Why not? There are cases where you want the promotion to happen ASAP, and there are cases where you don't care. But there are no scenarios where you want promotion to be slow, 5. Docs changes are missing. Here's what I think should be done: 1. Remove the check that prev checkpoint record exists. 2. Always do fast promotion if in standby mode. Remove the pg_ctl option. 3. Improve docs. - Heikki
On 6 February 2013 16:36, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > On 31.01.2013 21:33, Simon Riggs wrote: >> >> If anyone really wants me to revert, pls start new hackers thread to >> discuss, or comment on changes. > > > Yes, I still think this needs fixing or reverting. Let me reiterate my > my complaints: I'm sorry that they are complaints rather than just feedback, and will work to address them. > 1.3. Why check for the "prev" checkpoint? The latest checkpoint is > enough to recover, so why insist that also the previous one is present, > too? That was there from Kyotaro's patch and I left it as it was since it had been reviewed prior to me. I thought it was OK too, but now I think your arguments are good and I'm now happy to change to just the last checkpoint. That does bring into question what the value of the prev checkpoint is in any situation, not just this one... > There are normal scenarios where it won't be, like just after > recovering from a base backup. I consider it a bug that fast promotion > doesn't work right after restoring from a base backup. OK > 2. I don't like demoting the trigger file method to a second class > citizen. I think we should make all functionality available through both > methods. If there was a good reason for deprecating the trigger file > method, I could live with that, but this patch is not such a reason. I don't understand why we introduced a second method if they both will continue to be used. I see no reason for that, other than backwards compatibility. Enhancing both mechanisms suggests both will be supported into the future. Please explain why the second mode exists? > 3. I don't like conflating the promotion modes and shutdown modes in the > pg_ctl option. Shutdown modes and promotion modes are separate concepts. > The "fast" option is pretty clear, but why does "smart" mean "create an > immediate checkpoint before promotion"? How is that smarter than the > fast mode? > The "pg_ctl --help" on that is a bit confusing too: > >> Options for stop, restart or promote: -m, --mode=MODE MODE can >> be "smart", "fast", or "immediate" > > > The "immediate" mode is not actually valid for "pg_ctl promote". That is > clarified later in the output by listing out what the modes mean, but > that above line is misleading, We can always rename them, as you wish. > 4. I think fast promotion should be the default. Why not? There are > cases where you want the promotion to happen ASAP, and there are cases > where you don't care. But there are no scenarios where you want > promotion to be slow, Not true. Slow means safe and stable, and there are many scenarios where we want safe and stable. (Of course, nobody specifically requests slow). My feeling is that this is an area of exposure that we have no need and therefore no business touching. I will of course go with what others think here, but I don't find the argument that we should go fast always personally convincing. I am willing to relax it over time once we get zero field problems as a result. > 5. Docs changes are missing. OK > Here's what I think should be done: > > 1. Remove the check that prev checkpoint record exists. Agreed > 2. Always do fast promotion if in standby mode. Remove the pg_ctl option. Disagreed, other viewpoints welcome. > 3. Improve docs. Agreed -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
От
"Joshua D. Drake"
Дата:
On 02/06/2013 09:43 AM, Simon Riggs wrote: >> 4. I think fast promotion should be the default. Why not? There are >> cases where you want the promotion to happen ASAP, and there are cases >> where you don't care. But there are no scenarios where you want >> promotion to be slow, > > Not true. Slow means safe and stable, and there are many scenarios > where we want safe and stable. (Of course, nobody specifically > requests slow). My feeling is that this is an area of exposure that we > have no need and therefore no business touching. I will of course go > with what others think here, but I don't find the argument that we > should go fast always personally convincing. I am willing to relax it > over time once we get zero field problems as a result. Promotion, should by default should take the most safe, stable route and only that route. +1 On Simon's response. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC @cmdpromptinc - 509-416-6579
On Wed, Feb 6, 2013 at 12:43 PM, Simon Riggs <simon@2ndquadrant.com> wrote: >> 2. I don't like demoting the trigger file method to a second class >> citizen. I think we should make all functionality available through both >> methods. If there was a good reason for deprecating the trigger file >> method, I could live with that, but this patch is not such a reason. > > I don't understand why we introduced a second method if they both will > continue to be used. I see no reason for that, other than backwards > compatibility. Enhancing both mechanisms suggests both will be > supported into the future. Please explain why the second mode exists? I agree that we should be pushing people towards pg_ctl promote. I have no strong opinion about whether backward-compatibility for the trigger file method is a good idea or not. It might be a little soon to relegate that to second-class status, but I'm not sure. >> 4. I think fast promotion should be the default. Why not? There are >> cases where you want the promotion to happen ASAP, and there are cases >> where you don't care. But there are no scenarios where you want >> promotion to be slow, > > Not true. Slow means safe and stable, and there are many scenarios > where we want safe and stable. (Of course, nobody specifically > requests slow). My feeling is that this is an area of exposure that we > have no need and therefore no business touching. I will of course go > with what others think here, but I don't find the argument that we > should go fast always personally convincing. I am willing to relax it > over time once we get zero field problems as a result. I'm skeptical of the idea that we shouldn't default to fast-promote because the fast-promote code might be buggy. We do sometimes default new features to off on the grounds that they might be buggy - Hot Standby got an on/off switch partly for that reason - but usually we only add a knob if there's some plausible reason for wanting to change the setting independently of the possibility of bugs. For instance, in the case of Hot Standby, another of the reasons for adding a knob was that people wanted a way to make sure that they wouldn't accidentally connect to the standby when they intended to connect to the master. That may or may not have been a sufficiently *good* reason, but it was accepted as justification at the time. So I would ask this question: why would someone want to turn off fast-promote mode, assuming for the sake of argument that it isn't buggy? I think there might be good reasons to do that, but I'm not sure what they are. I doubt it will be a common thing to want. I think most people are going to want fast-promote, but many may not know enough to request it, which means that if it isn't the default, the code may not get much testing anyway. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
От
Heikki Linnakangas
Дата:
On 06.02.2013 20:02, Robert Haas wrote: > On Wed, Feb 6, 2013 at 12:43 PM, Simon Riggs<simon@2ndquadrant.com> wrote: >>> 2. I don't like demoting the trigger file method to a second class >>> citizen. I think we should make all functionality available through both >>> methods. If there was a good reason for deprecating the trigger file >>> method, I could live with that, but this patch is not such a reason. >> >> I don't understand why we introduced a second method if they both will >> continue to be used. I see no reason for that, other than backwards >> compatibility. Enhancing both mechanisms suggests both will be >> supported into the future. Please explain why the second mode exists? > > I agree that we should be pushing people towards pg_ctl promote. I > have no strong opinion about whether backward-compatibility for the > trigger file method is a good idea or not. It might be a little soon > to relegate that to second-class status, but I'm not sure. Both the trigger file and pg_ctl promote methods are useful in different setups. If you point the trigger file on an NFS mount or similar, that allows triggering promotion from a different host without providing shell access. You might want to put the trigger file on an NFS mount that also contains the WAL archive, for example. A promotion script that also controls the network routers to redirect traffic and STONITH the dead node, can then simply "touch /mnt/.../trigger" to promote. Sure, it could also ssh to the server and run "pg_ctl promote", but that requires more setup. - Heikki
On Wed, Feb 6, 2013 at 1:47 PM, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > On 06.02.2013 20:02, Robert Haas wrote: >> >> On Wed, Feb 6, 2013 at 12:43 PM, Simon Riggs<simon@2ndquadrant.com> >> wrote: >>>> >>>> 2. I don't like demoting the trigger file method to a second class >>>> citizen. I think we should make all functionality available through both >>>> methods. If there was a good reason for deprecating the trigger file >>>> method, I could live with that, but this patch is not such a reason. >>> >>> >>> I don't understand why we introduced a second method if they both will >>> continue to be used. I see no reason for that, other than backwards >>> compatibility. Enhancing both mechanisms suggests both will be >>> supported into the future. Please explain why the second mode exists? >> >> >> I agree that we should be pushing people towards pg_ctl promote. I >> have no strong opinion about whether backward-compatibility for the >> trigger file method is a good idea or not. It might be a little soon >> to relegate that to second-class status, but I'm not sure. > > > Both the trigger file and pg_ctl promote methods are useful in different > setups. If you point the trigger file on an NFS mount or similar, that > allows triggering promotion from a different host without providing shell > access. You might want to put the trigger file on an NFS mount that also > contains the WAL archive, for example. A promotion script that also controls > the network routers to redirect traffic and STONITH the dead node, can then > simply "touch /mnt/.../trigger" to promote. Sure, it could also ssh to the > server and run "pg_ctl promote", but that requires more setup. Good point. I hadn't thought about that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 6 February 2013 18:02, Robert Haas <robertmhaas@gmail.com> wrote: > So I would ask this question: why would someone want to turn off > fast-promote mode, assuming for the sake of argument that it isn't > buggy? You can write a question many ways, and lead people towards a conclusion as a result. Why would someone want to turn off safe-promote mode, assuming it was fast enough? -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
От
Heikki Linnakangas
Дата:
On 07.02.2013 10:41, Simon Riggs wrote: > On 6 February 2013 18:02, Robert Haas<robertmhaas@gmail.com> wrote: > >> So I would ask this question: why would someone want to turn off >> fast-promote mode, assuming for the sake of argument that it isn't >> buggy? > > You can write a question many ways, and lead people towards a > conclusion as a result. > > Why would someone want to turn off safe-promote mode, assuming it was > fast enough? Okay, I'll bite.. Because in some of your servers, the safe/slow promotion is not fast enough, and you want to use the same promotion script in both scenarios, to keep things simple. Because you're not sure if it's fast enough, and want to play it safe. Because faster is nicer, even if the slow mode would be "fast enough". It makes me uncomfortable that we're adding switches to pg_ctl promote just because we're worried there might be bugs in our code. If we don't trust the code as it is, it needs more testing. We can analyze the code more thoroughly, to make an educated guess on what's likely to happen if it's broken, and consider adding some sanity checks etc. to make the consequences less severe. We should not put the burden on our users to decide if the code is trustworthy enough to use. Note that we still wouldn't do fast promotion in crash recovery, so there's that escape hatch if there is indeed a bug in our code and fast promotion fails. - Heikki
On 7 February 2013 09:04, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > It makes me uncomfortable that we're adding switches to pg_ctl promote just > because we're worried there might be bugs in our code. If we don't trust the > code as it is, it needs more testing. We can analyze the code more > thoroughly, to make an educated guess on what's likely to happen if it's > broken, and consider adding some sanity checks etc. to make the consequences > less severe. We should not put the burden on our users to decide if the code > is trustworthy enough to use. I don't think I said I was worried about bugs in code, did I? The point is that this has been a proven mechanism for many years and we're now discussing turning that off completely with no user option to put it back, which has considerable risk with it. Acknowledging risks and taking risk mitigating actions is a normal part of any IT project. If we start getting unexplained errors it could take a long time to trace that back to the lack of a shutdown checkpoint. I don't mind saying openly this worries me and its why I took months to commit it. If there was no risk here and its all so easy, why didn't we commit this last year, or why didn't you override me and commit this earlier in this cycle? I have to say I care very little for the beauty or lack of command switches, in such a case. The "cost" there is low. Tell me you understand the risk I am discussing, tell me in your opinion we're safe and I'm being unnecessarily cautious, maybe even foolishly so, and I'll relent. I'll stand by that and take the flak. But saying you don't like a switch is like telling me you don't like the colour of my car safety belt. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Thu, Feb 7, 2013 at 4:04 AM, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > It makes me uncomfortable that we're adding switches to pg_ctl promote just > because we're worried there might be bugs in our code. If we don't trust the > code as it is, it needs more testing. We can analyze the code more > thoroughly, to make an educated guess on what's likely to happen if it's > broken, and consider adding some sanity checks etc. to make the consequences > less severe. We should not put the burden on our users to decide if the code > is trustworthy enough to use. +1 -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [COMMITTERS] pgsql: Fast promote mode skips checkpoint at end of recovery.
От
Kevin Grittner
Дата:
Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > On 07.02.2013 10:41, Simon Riggs wrote: >> Why would someone want to turn off safe-promote mode, assuming it was >> fast enough? > Because faster is nicer, even if the slow mode would be "fast enough". http://www.youtube.com/watch?v=H3R-rtWPyJY -Kevin
On 6 February 2013 17:43, Simon Riggs <simon@2ndquadrant.com> wrote: >> Here's what I think should be done: >> >> 1. Remove the check that prev checkpoint record exists. > > Agreed Done >> 2. Always do fast promotion if in standby mode. Remove the pg_ctl option. > > Disagreed, other viewpoints welcome. Waiting for further comments. >> 3. Improve docs. > > Agreed Pending. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services