Re: bgw_type (was Re: Why does logical replication launcher set application_name?)

On 9/27/17 18:59, Daniel Gustafsson wrote:
> The patch applies with minor fuzz, compiles without introduced warnings and
> work the way it says on the tin. The utility of the proposed functionality is
> a clear win so +1 on getting that in.

I have committed this patch incorporating the feedback in this thread.

> Regarding the following hunk:
>
> + process listing, for example. <structfield>bgw_name</structfield> on the
> + other hand can contain additional information about the specific process.
> + (Typically, the string for <structfield>bgw_name</structfield> will contain
> + the string for <structfield>bgw_type</structfield> somehow, but that is not
> + strictly required.)
>
> This reads a bit too (oddly) detailed to me, I would’ve phrased it as something
> along the lines of:
>
> "Typically, the string for bgw_name will contain the type somehow, but that
> is not strictly required.”

Yes, that's better.

> I find omitting “background worker” in the following hunk is leaving out
> valuable information for bgworkers with badly configured types, but it’s
> definitely a matter of taste rather than a straight-up patch critique:
>
> - errmsg("terminating background worker \"%s\" due to administrator command",
> - MyBgworkerEntry->bgw_name)));
> + errmsg("terminating %s due to administrator command",
> + MyBgworkerEntry->bgw_type)));

Also changed.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services