Re: shared-memory based stats collector - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: shared-memory based stats collector
Date
Msg-id 20200310.122725.1482409768204993835.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: shared-memory based stats collector  (Andres Freund <andres@anarazel.de>)
Responses Re: shared-memory based stats collector
Re: shared-memory based stats collector
List pgsql-hackers
Thank you all for the suggestions.

At Mon, 9 Mar 2020 12:25:39 -0700, Andres Freund <andres@anarazel.de> wrote in 
> Hi,
> 
> On 2020-03-09 15:04:23 -0400, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > On 2020-03-09 15:37:05 -0300, Alvaro Herrera wrote:
> > >> I'm worried that we're causing all processes to terminate when an
> > >> archiver dies in some ugly way; but in the current coding, it's pretty
> > >> harmless and we'd just start a new one.  I think this needs to be
> > >> reconsidered.  As far as I know, pgarchiver remains unconnected to
> > >> shared memory so a crash-restart cycle is not necessary.  We should
> > >> continue to just log the error message and move on.
> > 
> > > Why is it worth having the archiver be "robust" that way?
> > 
> > I'd ask a different question: what the heck is this patchset doing
> > touching the archiver in the first place?  I can see no plausible
> > reason for that doing anything related to stats collection.
> 
> As of a release or two back, it sends stats messages for archiving
> events:
> 
>             if (pgarch_archiveXlog(xlog))
>             {
>                 /* successful */
>                 pgarch_archiveDone(xlog);
> 
>                 /*
>                  * Tell the collector about the WAL file that we successfully
>                  * archived
>                  */
>                 pgstat_send_archiver(xlog, false);
> 
>                 break;            /* out of inner retry loop */
>             }
>             else
>             {
>                 /*
>                  * Tell the collector about the WAL file that we failed to
>                  * archive
>                  */
>                 pgstat_send_archiver(xlog, true);
> 
> 
> > If we now need some new background processing for stats, let's make a
> > new postmaster child process to do that, not overload the archiver
> > with unrelated responsibilities.
> 
> I don't think that's what's going on. It's just changing the archiver so
> it can report stats via shared memory - which subsequently means that it
> it can report stats via shared memory - which subsequently means that it
> needs to deal differently with errors than when it wasn't connected.

That's true, but I have the same concern with Tom. The archive bacame
too-tightly linked with other processes than actual relation. We may
need the second static shared memory segment apart from the current
one.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



pgsql-hackers by date:

Previous
From: Dilip Kumar
Date:
Subject: Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: shared-memory based stats collector