Re: log_min_messages per backend type - Mailing list pgsql-hackers
| From | Japin Li |
|---|---|
| Subject | Re: log_min_messages per backend type |
| Date | |
| Msg-id | MEAPR01MB3031FA1986F3FC91481E28CCB68DA@MEAPR01MB3031.ausprd01.prod.outlook.com Whole thread Raw |
| In response to | Re: log_min_messages per backend type ("Euler Taveira" <euler@eulerto.com>) |
| List | pgsql-hackers |
Hi Euler,
On Thu, 15 Jan 2026 at 13:07, "Euler Taveira" <euler@eulerto.com> wrote:
> On Tue, Dec 9, 2025, at 3:24 PM, Alvaro Herrera wrote:
>> On 2025-Dec-09, Alvaro Herrera wrote:
>>
>>> So here's your v6 again with those fixes as 0003 -- let's see what CI
>>> thinks of this. I haven't looked at your doc changes yet.
>>
>> This passed CI, so I have marked it as Ready for Committer. Further
>> comments are still welcome, of course, but if there are none, I intend
>> to get it committed in a few days.
>>
>
> I took another look after Chao Li comments [1]. I created the 0003 patch
> that does the sort as suggested. I think it is good to be consistent but
> I'm fine if we decided the additional code is not worth. The 32 in the
> MAX_LMM_STR_LEN is arbitrary but it is based on the size of the largest
> element in the list ("dead-end client backend:warning"). I didn't take
> into account the comma and space between elements but it is not
> necessary since other elements are smaller than the largest one.
> I didn't implement the 2nd suggestion.
>
> I also merged Alvaro's fix to 0002. The v8 is attached.
>
> I didn't change the commit message but if 0003 is merged into 0001 then
> it should mention that
>
> 8<--------------------------------------------------------------------8<
> The SHOW command presents well-formatted list sorted by process type and
> the generic log level is the first element list. It improves readability
> and has a clear indentation.
> 8<--------------------------------------------------------------------8<
>
> Do we really need a different backend type in this case? For background
> workers the description is "background worker". Shoundn't it use the
> same description for this edge case too?
>
> - backend_type_str = MyBgworkerEntry->bgw_type;
> + {
> + if (MyBgworkerEntry && MyBgworkerEntry->bgw_type[0] != '\0')
> + backend_type_str = MyBgworkerEntry->bgw_type;
> + else
> + backend_type_str = "early bgworker";
> + }
>
> I also noticed that commit 18d67a8d7d30 forgot to add gettext_noop to
> the get_backend_type_for_log function. It should be consistent with
> GetBackendTypeDesc() return.
>
>
> [1] https://postgr.es/m/1130A10B-E7AE-49F3-9E13-2CD69B3F9DFD@gmail.com
>
>
Thanks for updating the patch set.
Here are some comments.
1.
We can replace foreach with foreach_ptr in both v8-0001 and v8-0003.
2.
+/* log_min_messages */
+extern PGDLLIMPORT const char *const log_min_messages_process_types[];
+
Comment looks wrong.
3.
For cases where the process type is valid but the log level is unrecognized,
I suggest improving the error message for better clarity, e.g.:
Unrecognized log level "bar" for process type "backend"
4.
The function name string_cmp feels too generic.
Could we consider a more specific name, for example list_log_min_message_cmp
or another more appropriate one?
> --
> Euler Taveira
> EDB https://www.enterprisedb.com/
>
> [2. text/x-patch; v8-0001-log_min_messages-per-process-type.patch]...
>
> [3. text/x-patch; v8-0002-Assign-backend-type-earlier.patch]...
>
> [4. text/x-patch; v8-0003-fixup-log_min_messages-per-process-type.patch]...
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
pgsql-hackers by date: