Thread: Hot standby, pausing recovery
This if-block is misplaced: > case RECOVERY_TARGET_STOP_IMMEDIATE: > case RECOVERY_TARGET_STOP_XID: > case RECOVERY_TARGET_STOP_TIME: > paused = false; > break; > > /* > * If we're paused, and mode has changed reset to allow new settings > * to apply and maybe allow us to continue. > */ > if (paused && prevRecoveryTargetMode != recoveryTargetMode) > paused = false; > > case RECOVERY_TARGET_PAUSE_XID: Where was that supposed to go? In advance-mode, we will merrilly skip over a WAL record that's a recovery stop target. Is that a bug or a feature? If you pause recovery, and then continue, we reset to "target none" mode, even if a stopping point was set previously. That doesn't seem right to me. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Fri, 2009-10-16 at 09:33 +0300, Heikki Linnakangas wrote: > In advance-mode, we will merrilly skip over a WAL record that's a > recovery stop target. Is that a bug or a feature? Merrily?!? I saw it more as a sombre stepping motion. Advance currently means set target at next record and continue. Only one target is currently supported, which means there is no other recovery stop target. > If you pause recovery, and then continue, we reset to "target none" > mode, even if a stopping point was set previously. Yes, currently. Resetting the target mode will do what you want, rather than continue. > That doesn't seem right to me. We can define these things various ways and I'm happy to change it to be the way people find most intuitive and useful. You're the first person to give me feedback on how these things should work, so I'm happy to hear how you would like it to work. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs wrote: > On Fri, 2009-10-16 at 09:33 +0300, Heikki Linnakangas wrote: >> If you pause recovery, and then continue, we reset to "target none" >> mode, even if a stopping point was set previously. > > Yes, currently. Resetting the target mode will do what you want, rather > than continue. > >> That doesn't seem right to me. > > We can define these things various ways and I'm happy to change it to be > the way people find most intuitive and useful. You're the first person > to give me feedback on how these things should work, so I'm happy to > hear how you would like it to work. Thinking about the interface a bit more: pg_recovery_advance() isn't very useful except for debugging hot standby itself. Normal users don't care about individual WAL records, since the effect of record X isn't visible until the transaction commit record anyway. A function to advance to next *commit* record would make a lot more sense, allowing you to step through transactions one at a time. A function to step forward X minutes would be useful too. At this point, I'd like to cut out all those control functions to pause/stop at various points from the patch. They're not required for Hot Standby, and the less stuff I have to review right now the better. Don't get me wrong, I absolutely love those functions, it's a great feature that I do want to get in before release, but we need more discussion on the exact set of functions and how they should work. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Tue, 2009-10-20 at 10:33 +0300, Heikki Linnakangas wrote: > At this point, I'd like to cut out all those control functions to > pause/stop at various points from the patch. OK -- Simon Riggs www.2ndQuadrant.com
On Tue, 2009-10-20 at 09:43 +0100, Simon Riggs wrote: > On Tue, 2009-10-20 at 10:33 +0300, Heikki Linnakangas wrote: > > At this point, I'd like to cut out all those control functions to > > pause/stop at various points from the patch. > > OK Maybe OK, I should say. Some parts are important for testing HS itself and for debugging situations that occur. -- Simon Riggs www.2ndQuadrant.com