Re: please update ps display for recovery checkpoint

2021-06-09 Thread Michael Paquier
On Sun, Jun 06, 2021 at 09:13:48PM -0500, Justin Pryzby wrote: > Putting this into fd.c seems to assume that we can clobber "ps", which is fine > when called by StartupXLOG(), but it's a public interface, so I'm not sure if > it's okay to assume that's the only caller. Maybe it should check if >

Re: please update ps display for recovery checkpoint

2021-06-09 Thread Michael Paquier
On Mon, Jun 07, 2021 at 01:28:06PM -0400, Robert Haas wrote: > On Mon, Jun 7, 2021 at 12:02 PM Bossart, Nathan wrote: >> I've seen a few functions cause lengthy startups, including >> SyncDataDirectory() (for which I was grateful to see 61752afb), >> StartupReorderBuffer(), and

Re: please update ps display for recovery checkpoint

2021-06-07 Thread Robert Haas
On Mon, Jun 7, 2021 at 12:02 PM Bossart, Nathan wrote: > On 6/6/21, 7:14 PM, "Justin Pryzby" wrote: > > Now, I wonder whether the startup process should also include some detail > > about > > "syncing data dir". It's possible to strace the process to see what it's > > doing, but most DBA would

Re: please update ps display for recovery checkpoint

2021-06-07 Thread Bossart, Nathan
On 6/6/21, 7:14 PM, "Justin Pryzby" wrote: > Now, I wonder whether the startup process should also include some detail > about > "syncing data dir". It's possible to strace the process to see what it's > doing, but most DBA would probably not know that, and it's helpful to know the > status of

Re: please update ps display for recovery checkpoint

2021-06-06 Thread Justin Pryzby
On Mon, Dec 14, 2020 at 12:01:33PM +0900, Michael Paquier wrote: > On Sat, Dec 12, 2020 at 12:41:25AM +, Bossart, Nathan wrote: > > On 12/11/20, 4:00 PM, "Michael Paquier" wrote: > >> My counter-proposal is like the attached, with the set/reset part not > >> reversed this time, and the code

Re: please update ps display for recovery checkpoint

2020-12-13 Thread Michael Paquier
On Sun, Dec 13, 2020 at 09:22:24PM -0600, Justin Pryzby wrote: > I'm not sure, but we could consider backpatching something to clear the > "recovering NNN" that's currently displayed during checkpoint, even though > recovery of NNN has already completed. Possibly just calling >

Re: please update ps display for recovery checkpoint

2020-12-13 Thread Justin Pryzby
On Mon, Dec 14, 2020 at 12:01:33PM +0900, Michael Paquier wrote: > On Sat, Dec 12, 2020 at 12:41:25AM +, Bossart, Nathan wrote: > > On 12/11/20, 4:00 PM, "Michael Paquier" wrote: > >> My counter-proposal is like the attached, with the set/reset part not > >> reversed this time, and the code

Re: please update ps display for recovery checkpoint

2020-12-13 Thread Michael Paquier
On Sat, Dec 12, 2020 at 12:41:25AM +, Bossart, Nathan wrote: > On 12/11/20, 4:00 PM, "Michael Paquier" wrote: >> My counter-proposal is like the attached, with the set/reset part not >> reversed this time, and the code indented :p > > Haha. LGTM. Thanks. I have applied this one, then. --

Re: please update ps display for recovery checkpoint

2020-12-11 Thread Bossart, Nathan
On 12/11/20, 4:00 PM, "Michael Paquier" wrote: > On Fri, Dec 11, 2020 at 06:54:42PM +, Bossart, Nathan wrote: >> This approach seems reasonable to me. I've attached my take on it. > > + /* Reset the process title */ > + set_ps_display(""); > I would still recommend to avoid

Re: please update ps display for recovery checkpoint

2020-12-11 Thread Michael Paquier
On Fri, Dec 11, 2020 at 06:54:42PM +, Bossart, Nathan wrote: > This approach seems reasonable to me. I've attached my take on it. + /* Reset the process title */ + set_ps_display(""); I would still recommend to avoid calling set_ps_display() if there is no need to so as we avoid

Re: please update ps display for recovery checkpoint

2020-12-10 Thread Michael Paquier
On Thu, Dec 10, 2020 at 10:02:10PM -0600, Justin Pryzby wrote: > Isn't the sense of "reset" inverted ? It is ;p -- Michael signature.asc Description: PGP signature

Re: please update ps display for recovery checkpoint

2020-12-10 Thread Justin Pryzby
Isn't the sense of "reset" inverted ? I think maybe you mean to do set_ps_display(""); in the "if reset". On Fri, Dec 11, 2020 at 12:54:22PM +0900, Michael Paquier wrote: > +update_checkpoint_display(int flags, bool restartpoint, bool reset) > +{ > + if (reset) > + { > + char

Re: please update ps display for recovery checkpoint

2020-12-10 Thread Michael Paquier
On Wed, Dec 09, 2020 at 06:37:22PM +, Bossart, Nathan wrote: > On 12/8/20, 10:16 PM, "Michael Paquier" wrote: >> Yeah, I'd rather leave out the part of the patch where we have the >> checkpointer say "idle". So I still think that what v3 did upthread, >> by just resetting the ps display in

Re: please update ps display for recovery checkpoint

2020-12-08 Thread Michael Paquier
On Wed, Dec 09, 2020 at 02:00:44AM +0900, Fujii Masao wrote: > I agree it might be helpful to display something like > "checkpointing" or "waiting for checkpoint" in PS title for the > startup process. Thanks. > But, at least for me, it seems strange to display "idle" only for > checkpointer.

Re: please update ps display for recovery checkpoint

2020-12-08 Thread Fujii Masao
On 2020/12/05 2:17, Bossart, Nathan wrote: On 12/3/20, 1:19 PM, "Bossart, Nathan" wrote: I like the idea behind this patch. I wrote a new version with a couple of changes: I missed an #include in this patch. Here's a new one with that fixed. I agree it might be helpful to display

Re: please update ps display for recovery checkpoint

2020-12-03 Thread Bossart, Nathan
On 12/3/20, 1:58 PM, "Justin Pryzby" wrote: > On Thu, Dec 03, 2020 at 09:18:07PM +, Bossart, Nathan wrote: >> I considered also checking that update_process_title was enabled, but >> I figured that these ps display updates should happen sparsely enough >> that it wouldn't make much of an

Re: please update ps display for recovery checkpoint

2020-12-03 Thread Justin Pryzby
On Thu, Dec 03, 2020 at 09:18:07PM +, Bossart, Nathan wrote: > I considered also checking that update_process_title was enabled, but > I figured that these ps display updates should happen sparsely enough > that it wouldn't make much of an impact. Since bf68b79e5, update_ps_display is

Re: please update ps display for recovery checkpoint

2020-10-02 Thread Justin Pryzby
On Fri, Oct 02, 2020 at 04:28:14PM +0900, Michael Paquier wrote: > > Related: I have always thought that this message meant "recovery will > > complete > > Real Soon", but I now understand it to mean "beginning the recovery > > checkpoint, > > which is flagged CHECKPOINT_IMMEDIATE" (and may take

Re: please update ps display for recovery checkpoint

2020-10-02 Thread Michael Paquier
On Sat, Sep 19, 2020 at 11:00:31AM -0500, Justin Pryzby wrote: > Maybe it's a bad idea if the checkpointer is continuously changing its > display. > I don't see the utility in it, since log_checkpoints does more than ps could > ever do. I'm concerned that would break things for someone using

Re: please update ps display for recovery checkpoint

2020-09-19 Thread Justin Pryzby
On Thu, Sep 10, 2020 at 01:37:10PM +0900, Michael Paquier wrote: > On Wed, Sep 09, 2020 at 09:00:50PM -0500, Justin Pryzby wrote: > > What would you want the checkpointer's ps to say ? > > > > Normally it just says: > > postgres 3468 3151 0 Aug27 ?00:20:57 postgres: checkpointer

Re: please update ps display for recovery checkpoint

2020-09-09 Thread Michael Paquier
On Wed, Sep 09, 2020 at 09:00:50PM -0500, Justin Pryzby wrote: > What would you want the checkpointer's ps to say ? > > Normally it just says: > postgres 3468 3151 0 Aug27 ?00:20:57 postgres: checkpointer > Note that CreateCheckPoint() can also be

Re: please update ps display for recovery checkpoint

2020-09-09 Thread Justin Pryzby
On Mon, Aug 31, 2020 at 03:52:44PM +0900, Michael Paquier wrote: > On Thu, Aug 20, 2020 at 05:09:05PM +0900, Michael Paquier wrote: > > That could be helpful. Wouldn't it be better to use "end-of-recovery > > checkpoint" instead? That's the common wording in the code comments. > > > > I don't

Re: please update ps display for recovery checkpoint

2020-08-31 Thread Michael Paquier
On Thu, Aug 20, 2020 at 05:09:05PM +0900, Michael Paquier wrote: > That could be helpful. Wouldn't it be better to use "end-of-recovery > checkpoint" instead? That's the common wording in the code comments. > > I don't see the point of patch 0002. In the same paragraph, we > already know that

Re: please update ps display for recovery checkpoint

2020-08-20 Thread Michael Paquier
On Wed, Aug 19, 2020 at 12:20:50AM +, k.jami...@fujitsu.com wrote: > On Wednesday, August 19, 2020 7:53 AM (GMT+9), Justin Pryzby wrote: >> During crash recovery, the server writes this to log: >> Please change to say "recovery checkpoint" or similar, as I mentioned here. >>

RE: please update ps display for recovery checkpoint

2020-08-18 Thread k.jami...@fujitsu.com
On Wednesday, August 19, 2020 7:53 AM (GMT+9), Justin Pryzby wrote: Hi, All the patches apply, although when applying them the following appears: (Stripping trailing CRs from patch; use --binary to disable.) > During crash recovery, the server writes this to log: > > < 2020-08-16

please update ps display for recovery checkpoint

2020-08-18 Thread Justin Pryzby
During crash recovery, the server writes this to log: < 2020-08-16 08:46:08.601 -03 >LOG: redo done at 2299C/1EC6BA00 < 2020-08-16 08:46:08.877 -03 >LOG: checkpoint starting: end-of-recovery immediate But runs a checkpoint, which can take a long time, while the "ps" display still says