Re: max_standby_delay considered harmful

On May 9, 2010, at 13:59 , Dimitri Fontaine wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>> I like the proposal of a boolean because it provides only the minimal
>> feature set of two cases that are both clearly needed and easily
>> implementable. Whatever we do later is certain to provide a superset
>> of those two cases. If we do something else (and that includes my own
>> proposal of a straight lock timeout), we'll be implementing something
>> we might wish to take back later. Taking out features after they've
>> been in a release is very hard, even if we realize they're badly
>> designed.
>
> That's where I though my proposal fitted in. I fail to see us wanting to
> take back explicit pause/resume admin functions in any future release.
>
> Now, after having read Greg's arguments, my vote would be the following:
> - hot_standby_conflict_winner = queries|replay, defaults to replay
> - add pause/resume so that people can switch temporarily to queries
> - label max_standby_delay *experimental*, keep current code

Adding pause/resume seems to introduce some non-trivial locking problems, though. How would you handle a pause request if the recovery process currently held a lock?

Dropping the lock is not an option for correctness reasons. Otherwise you wouldn't have needed to take the lock in the first place, no?

Pausing with the lock held leads to priority-inversion like problems. Queries now might block until recovery is resumed - quite the opposite of what pause() is supposed to archive

The only remaining option is to continue applying WAL until you reach a point where no locks are held, then pause. But from a user's POV that is nearly indistinguishable from simply setting hot_standby_conflict_winner to in the first place I think.

best regards,
Florian Pflug