Re: shared-memory based stats collector

Thank you all for the suggestions.

At Mon, 9 Mar 2020 12:25:39 -0700, Andres Freund <andres(at)anarazel(dot)de> wrote in
> Hi,
>
> On 2020-03-09 15:04:23 -0400, Tom Lane wrote:
> > Andres Freund <andres(at)anarazel(dot)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