On 21 December 2016 at 15:40, Ants Aasma <ants.aasma@eesti.ee> wrote:
>> So -1 on this part of the patch, unless there's something I've misunderstood.
>
> Currently there was no feedback sent if hot standby was not active. I
> was not sure if it was safe to call GetOldestXmin() in that case.
> However I did not consider cascading replica slots wanting to hold
> back xmin, where resetting the parents xmin is indeed wrong. Do you
> know if GetOldestXmin() is safe at this point and we can just remove
> the HotStandbyActive() check? Otherwise I think the correct approach
> is to move the check and return inside the hot_standby_feedback case
> like this:
>
> if (hot_standby_feedback)
> {
> if (!HotStandbyActive())
> return;
I feel like I'm missing something obvious here. If we force sending
hot standby feedback at least once, by assuming
master_has_standby_xmin = true at startup, why isn't that sufficient?
We'll send InvalidTransactionId if hot_standby_feedback is off. Isn't
that the point?
It's safe to call GetOldestXmin pretty much as soon as we load the
recovery start checkpoint. It won't consider the state of replication
slots until later in startup, but that's a pre-existing flaw that
should be addressed separately.
Why do we need to do more than master_has_standby_xmin = true ?
-- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services