On Thu, Dec 21, 2017 at 1:38 AM, Michael Paquier <michael.paqu...@gmail.com> wrote:
> On Wed, Dec 20, 2017 at 9:02 PM, Magnus Hagander <mag...@hagander.net> > wrote: > > On Wed, Dec 20, 2017 at 12:50 PM, Michael Paquier > > <michael.paqu...@gmail.com> wrote: > > Yes. Of course. I can't read. That's the same as the notice below about > it > > not returning false -- I managed to miss the extra if() there, and > thought > > it always exited with ERROR. > > I think that the call to pgstat_report_activity in WalSndLoop() should > be kept as well. There is a small gap between the moment the process > is started and the first replication command is run. > Eh. But WalSndLoop() is called *after* exec_replication_command(), isn't it? exec_replication_command() is called from PostgresMain(), and then calls WalSndLoop(). So I agree there is a small gap, but actually moving it to exec_replication_command() makes that gap smaller than it was before, no? > > >> + /* Report to pgstat that this process is running */ > >> + pgstat_report_activity(STATE_RUNNING, NULL); > >> Bonus points if cmd_string is used instead of string? This way, you > >> can know what is the replication command running ;) > > > > Do we want that though? That would be a compat break at least, wouldn't > it? > > Perhaps not, I found the idea funky but you actually don't want to > show a string in exec_replication_command for a T_SQLCmd node. That's > not complicated to check either. So let's discard such a thing for > now. > Agreed :) -- Magnus Hagander Me: https://www.hagander.net/ <http://www.hagander.net/> Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
