On Wed, Jan 13, 2021 at 3:27 PM Yugo NAGATA <nag...@sraoss.co.jp> wrote: > > On Thu, 10 Dec 2020 11:25:23 +0530 > Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > > However, I wonder users don't expect pg_is_wal_replay_paused to wait. > > > > Especially, if max_standby_streaming_delay is -1, this will be blocked > > > > forever, > > > > although this setting may not be usual. In addition, some users may set > > > > recovery_min_apply_delay for a large. If such users call > > > > pg_is_wal_replay_paused, > > > > it could wait for a long time. > > > > > > > > At least, I think we need some descriptions on document to explain > > > > pg_is_wal_replay_paused could wait while a time. > > > > > > Ok > > > > Fixed this, added some comments in .sgml as well as in function header > > Thank you for fixing this. > > Also, is it better to fix the description of pg_wal_replay_pause from > "Pauses recovery." to "Request to pause recovery." in according with > pg_is_wal_replay_paused?
Okay > > > > > Also, how about adding a new boolean argument to > > > > pg_is_wal_replay_paused to > > > > control whether this waits for recovery to get paused or not? By > > > > setting its > > > > default value to true or false, users can use the old format for > > > > calling this > > > > and the backward compatibility can be maintained. > > > > > > So basically, if the wait_recovery_pause flag is false then we will > > > immediately return true if the pause is requested? I agree that it is > > > good to have an API to know whether the recovery pause is requested or > > > not but I am not sure is it good idea to make this API serve both the > > > purpose? Anyone else have any thoughts on this? > > > > > I think the current pg_is_wal_replay_paused() already has another purpose; > this waits recovery to actually get paused. If we want to limit this API's > purpose only to return the pause state, it seems better to fix this to return > the actual state at the cost of lacking the backward compatibility. If we want > to know whether pause is requested, we may add a new API like > pg_is_wal_replay_paluse_requeseted(). Also, if we want to wait recovery to > actually > get paused, we may add an option to pg_wal_replay_pause() for this purpose. > > However, this might be a bikeshedding. If anyone don't care that > pg_is_wal_replay_paused() can make user wait for a long time, I don't care > either. I don't think that it will be blocked ever, because pg_wal_replay_pause is sending the WakeupRecovery() which means the recovery process will not be stuck on waiting for the WAL. > > > > As another comment, while pg_is_wal_replay_paused is blocking, I can > > > > not cancel > > > > the query. I think CHECK_FOR_INTERRUPTS() is necessary in the waiting > > > > loop. > > How about this fix? I think users may want to cancel > pg_is_wal_replay_paused() during > this is blocking. Yeah, we can do this. I will send the updated patch after putting some more thought into these comments. Thanks again for the feedback. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
