| From: | Japin Li <japinli(at)hotmail(dot)com> |
|---|---|
| To: | "Euler Taveira" <euler(at)eulerto(dot)com> |
| Cc: | "Alvaro Herrera" <alvherre(at)alvh(dot)no-ip(dot)org>, "Andres Freund" <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, "Chao Li" <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Subject: | Re: log_min_messages per backend type |
| Date: | 2026-01-16 05:54:44 |
| Message-ID: | MEAPR01MB3031FA1986F3FC91481E28CCB68DA@MEAPR01MB3031.ausprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Euler,
On Thu, 15 Jan 2026 at 13:07, "Euler Taveira" <euler(at)eulerto(dot)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.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alexander Lakhin | 2026-01-16 06:00:00 | Re: Bug in amcheck? |
| Previous Message | Amit Langote | 2026-01-16 05:52:49 | Re: Segmentation fault on proc exit after dshash_find_or_insert |