Re: shared-memory based stats collector

On 2021/03/12 13:49, Kyotaro Horiguchi wrote:
> At Fri, 12 Mar 2021 10:38:00 +0900 (JST), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in
>> At Fri, 12 Mar 2021 10:03:31 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
>>>> I moved archiver from the current location to next to "walsenders"
>>>> that is to be terminated along with archiver.
>>>> The attached the only 0003 of the new version based on the last one
>>>> from Fujii-san.
>>>
>>> Thanks for updating the patch! But you forgot to add the changes
>>> related to pgprocno into the patch?
>>
>> Although I intentinally didn't do that because "it really doesn't
>> matter", please wait for a while for the new version. I'm going to
>> make PgArchData have only one member procno instead of replacing the
>> struct with a single variable.
>
> I noticed that I accidentally removed the launch-suppression feature
> that is to avoid frequent relaunching. That mechanism is needed on
> the postmaster side. I added PgArchIsSuppressed() to do the same check
> with the old pgarch_start() and make it called as a part of
> PgArchStartupAllowed().

You're right! At least for me the function name PgArchIsSuppressed() sounds not good to me. What about something like PgArchCanRestart()?

This is not fault of this patch. But last_pgarch_start_time should be initialized with zero?

+ if ((curtime - last_pgarch_start_time) < PGARCH_RESTART_INTERVAL)
+ return true;

Why did you remove the cast to unsigned int there?

+ /*
+ * Advertise our latch that backends can use to wake us up while we're
+ * sleeping.
+ */
+ PgArch->pgprocno = MyProc->pgprocno;

The comment should be updated?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION