This commit introduced a problem with wal_keep_segments:
commit dfda6ebaec6763090fb78b458a979b558c50b39b
The problem seems to be that the underflow warned about is happening, because the check to guard it was checking the wrong thing. However, I don't really understand KeepLogSeg. It seems like segno, and hence recptr, don't actually serve any purpose.
Hmm, the check is actually correct, but the assignment in the else-branch isn't. The idea of KeepLogSeg is to calculate recptr - wal_keep_segments, and assign that to *logSegNo. But only if *logSegNo is not already < than the calculated value. Does the attached look correct to you?
Let me describe what I think is going on. My description is "On start, recptr is the redo location of the just-completed checkpoint, and logSegNo is the redo location segment of the checkpoint before that one. We want to keep the previous-checkpoint redo location, and we also want to keep wal_keep_segments before the current-checkpoint redo location, so we take whichever is earlier."
If my understanding is now correct, then I think your patch looks correct. (Also, applying it fixed the problem I was having.)
Why do we keep wal_keep_segments before the just-finished checkpoint, rather than keeping that many before the previous checkpoint? I seems like it would be more intuitive (to the DBA) for that parameter to mean "keep this many more segments than you otherwise would". I'm not proposing we change it, I'm just curious about why it is done that way.
