At Wed, 10 Mar 2021 21:47:51 +0900, Fujii Masao <masao.fu...@oss.nttdata.com> 
wrote in 
> Attached is the updated version of the 0003 patch. Barring any
> objection,
> I will commit this patch.
> 
> 
> -#include "storage/latch.h"
> -#include "storage/proc.h"
> 
> I removed these because they are no longer necessary.

Mmm. Sorry for the garbage.

>         <literal>logical replication worker</literal>,
>         <literal>parallel worker</literal>, <literal>background
>         writer</literal>,
>         <literal>client backend</literal>, <literal>checkpointer</literal>,
> +       <literal>archiver</literal>,
>         <literal>startup</literal>, <literal>walreceiver</literal>,
>         <literal>walsender</literal> and <literal>walwriter</literal>.
> 
> In the document about pg_stat_activity, possible values in
> backend_type
> column are all listed. I added "archiver" into the list.
> 
> BTW, those values were originally listed in alphabetical order,
> but ISTM that they were gotten out of order at a certain moment.
> So they should be listed in alphabetical order again. This should
> be implemented as a separate patch.

Thanks for adding it.

They are also mildly sorted by function or characteristics. I'm not
sure which is better, but anyway they should be the order based on a
clear criteria.

> -PgArchData *PgArch = NULL;
> +static PgArchData *PgArch = NULL;
> 
> I marked PgArchData as static because it's used only in pgarch.c.

Right.

> -             ShmemInitStruct("Archiver ", PgArchShmemSize(), &found);
> + ShmemInitStruct("Archiver Data", PgArchShmemSize(), &found);
> 
> I found that the original shmem name ended with unnecessary space
> character.
> I replaced it with "Archiver Data".

Oops. The trailing space is where I stopped writing the string and try
to find a better word, then in the meanwhile, my mind have been
attracted to something else and left.  I don't object to "Archiver
Data".  Thanks for completing it.

> In reaper(), I moved the code block for archiver to the original
> location.

Agreed.

> I ran pgindent for the files that the patch modifies.

Yeah, I forgot to add the new struct into typedefs.list.  I
intentionally omitted clearing newly-acquired shared memory but it
doesn't matter to do that.

So, I'm fine with it. Thanks for taking this.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


Reply via email to