From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Daniel Gustafsson <daniel(at)yesql(dot)se>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Printing backtrace of postgres processes |
Date: | 2021-11-15 05:04:00 |
Message-ID: | CALDaNm3m5hVB9CicU5-VzLgT6dvFYYJNyU=8U7OA9+2Wx6=Gqg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Nov 15, 2021 at 7:37 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Sun, Nov 14, 2021 at 8:49 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > > 7) Do we need TAP tests for this function? I think it is sufficient to
> > > test the function in misc_functions.sql, please remove
> > > 002_print_backtrace_validation.pl. Note that we don't do similar TAP
> > > testing for pg_log_backend_memory_contexts as well.
> >
> > I felt let's keep this test case, all the other tests just check if it
> > returns true or false, it does not checks for the contents in the
> > logfile. This is the only test which checks the logfile.
>
> I still don't agree to have test cases within a new file
> 002_print_backtrace_validation.pl. I feel this test case doesn't add
> value because the code coverage is done by .sql test cases and .pl
> just ensures the backtrace appears in the server logs. I don't think
> we ever need a new file for this purpose. If this is the case, then
> there are other functions like pg_log_backend_memory_contexts or
> pg_log_query_plan (in progress thread) might add the same test files
> for the same reasons which make the TAP tests i.e. "make check-world"
> to take longer times. Moreover, pg_log_backend_memory_contexts has
> been committed without having a TAP test case.
>
> I think we can remove it.
Removed
> Few more comments on v11:
> 1) I think we can improve here by adding a link to "backend" as well,
> I will modify it in the other thread.
> + Requests to log the backtrace of the backend or the
> + <glossterm linkend="glossary-wal-sender">WAL sender</glossterm> or
> Something like:
> + Requests to log the backtrace of the <glossterm
> linkend="glossary-backend">backend</glossterm> or the
> + <glossterm linkend="glossary-wal-sender">WAL sender</glossterm> or
Modified
> 2) I think "which is enough because the target process for logging of
> backtrace is a backend" isn't valid anymore with 0002, righit? Please
> remove it.
> + * to call this function if we see PrintBacktracePending set. It is called from
> + * CHECK_FOR_INTERRUPTS() or from process specific interrupt handlers, which is
> + * enough because the target process for logging of backtrace is a backend.
>
> > Thanks for the comments, v11 patch attached at [1] has the changes for the same.
Modified
Thanks for the comments, the attached v12 patch has the changes for the same.
Regards,
Vignesh
Attachment | Content-Type | Size |
---|---|---|
v12-0001-Print-backtrace-of-specified-postgres-process.patch | text/x-patch | 19.8 KB |
v12-0002-pg_print_backtrace-support-for-printing-backtrac.patch | text/x-patch | 10.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Bharath Rupireddy | 2021-11-15 05:30:41 | Re: Printing backtrace of postgres processes |
Previous Message | Justin Pryzby | 2021-11-15 03:56:33 | Re: Add psql command to list constraints |