Thanks Bharath for your review comments. Please find my comments inline below.
On Thu, Jan 28, 2021 at 7:40 PM Bharath Rupireddy
wrote:
>
> On Thu, Jan 28, 2021 at 5:22 PM vignesh C wrote:
> > Thanks for the comments, I have fixed and attached an updated patch
> > with the fixes for the same.
>
On Thu, Jan 28, 2021 at 5:22 PM vignesh C wrote:
> Thanks for the comments, I have fixed and attached an updated patch
> with the fixes for the same.
> Thoughts?
Thanks for the patch. Here are few comments:
1) I think it's return SIGNAL_BACKEND_SUCCESS; instead of return 0; in
check_valid_pid?
On Wed, Jan 27, 2021 at 10:40 PM Andres Freund wrote:
>
> Hi,
>
> On 2021-01-27 19:05:16 +0530, vignesh C wrote:
>
> > /*
> > + * LogBackTrace
> > + *
> > + * Get the backtrace and log the backtrace to log file.
> > + */
> > +void
> > +LogBackTrace(void)
> > +{
> > + int
Hi,
On 2021-01-27 19:05:16 +0530, vignesh C wrote:
> /*
> + * LogBackTrace
> + *
> + * Get the backtrace and log the backtrace to log file.
> + */
> +void
> +LogBackTrace(void)
> +{
> + int save_errno = errno;
> +
> + void *buf[100];
> + int
On Thu, Jan 21, 2021 at 7:26 AM Tom Lane wrote:
>
> Craig Ringer writes:
> > On Wed, 20 Jan 2021 at 05:23, Tom Lane wrote:
> >> BTW, it also looks like the patch is doing nothing to prevent the
> >> backtrace from being sent to the connected client.
>
> > I don't see a good reason to send a bt
On Wed, Jan 20, 2021 at 9:24 PM Craig Ringer
wrote:
> I know lots of this stuff can seem like a pointless sidetrack because the
> utility of it is not obvious on dev systems or when you're doing your own
> hands-on expert support on systems you own and operate yourself. These sorts
> of things
On Thu, 21 Jan 2021 at 09:56, Tom Lane wrote:
> Craig Ringer writes:
> > On Wed, 20 Jan 2021 at 05:23, Tom Lane wrote:
> >> BTW, it also looks like the patch is doing nothing to prevent the
> >> backtrace from being sent to the connected client.
>
> > I don't see a good reason to send a bt to
Craig Ringer writes:
> On Wed, 20 Jan 2021 at 05:23, Tom Lane wrote:
>> BTW, it also looks like the patch is doing nothing to prevent the
>> backtrace from being sent to the connected client.
> I don't see a good reason to send a bt to a client. Even though these
> backtraces won't be analysing
On Wed, 20 Jan 2021 at 05:23, Tom Lane wrote:
>
> Recursion is scary, but it should (I think) not be possible if this
> is driven off CHECK_FOR_INTERRUPTS. There will certainly be none
> of those in libbacktrace.
>
We can also hold interrupts for the call, and it might be wise to do so.
One
On Wed, 20 Jan 2021 at 01:31, Robert Haas wrote:
> On Sat, Jan 16, 2021 at 3:21 PM Tom Lane wrote:
> > I'd argue that backtraces for those processes aren't really essential,
> > and indeed that trying to make the syslogger report its own backtrace
> > is damn dangerous.
>
> I agree. Ideally I'd
On Wed, Jan 20, 2021 at 2:52 AM Tom Lane wrote:
>
> Robert Haas writes:
> > On Tue, Jan 19, 2021 at 12:50 PM Tom Lane wrote:
> >> I think it's got security hazards as well. If we restricted the
> >> feature to cause a trace of only one process at a time, and required
> >> that process to be
Robert Haas writes:
> On Tue, Jan 19, 2021 at 12:50 PM Tom Lane wrote:
>> I think it's got security hazards as well. If we restricted the
>> feature to cause a trace of only one process at a time, and required
>> that process to be logged in as the same database user as the
>> requestor (or at
On Tue, Jan 19, 2021 at 12:50 PM Tom Lane wrote:
> The thing that is scaring me the most is the "broadcast" aspect.
> For starters, I think that that is going to be useless in the
> field because of the likelihood that different backends' stack
> traces will get interleaved in whatever log file
Robert Haas writes:
> On Sat, Jan 16, 2021 at 3:21 PM Tom Lane wrote:
>> (Personally, I think this whole patch fails the safety-vs-usefulness
>> tradeoff, but I expect I'll get shouted down.)
> What do you see as the main safety risks here?
The thing that is scaring me the most is the
On Sat, Jan 16, 2021 at 3:21 PM Tom Lane wrote:
> I'd argue that backtraces for those processes aren't really essential,
> and indeed that trying to make the syslogger report its own backtrace
> is damn dangerous.
I agree. Ideally I'd like to be able to use the same mechanism
everywhere and
On Sun, Jan 17, 2021 at 10:26 PM vignesh C wrote:
>
> On Sat, Jan 16, 2021 at 11:10 PM Andres Freund wrote:
> >
> > Hi,
> >
> > On Sat, Jan 16, 2021, at 09:34, vignesh C wrote:
> > > On Sat, Jan 16, 2021 at 1:40 AM Andres Freund wrote:
> > > >
> > > > On 2021-01-15 09:53:05 +0100, Peter
On Mon, 18 Jan 2021 at 00:56, vignesh C wrote:
>
> Thanks for your comments Andres, I will ignore it for the processes
> which do not have access to ProcSignal. I will make the changes and
> post a patch for this soon.
>
I think that's sensible.
I've had variable results with glibc's
On Sat, Jan 16, 2021 at 11:10 PM Andres Freund wrote:
>
> Hi,
>
> On Sat, Jan 16, 2021, at 09:34, vignesh C wrote:
> > On Sat, Jan 16, 2021 at 1:40 AM Andres Freund wrote:
> > >
> > > On 2021-01-15 09:53:05 +0100, Peter Eisentraut wrote:
> > > > On 2020-12-08 10:38, vignesh C wrote:
> > > > > I
vignesh C writes:
> On Sat, Jan 16, 2021 at 1:40 AM Andres Freund wrote:
>> Why is a full signal needed? Seems the procsignal infrastructure should
>> suffice?
> Most of the processes have access to ProcSignal, for these processes
> printing of callstack signal was handled by using ProcSignal.
Hi,
On Sat, Jan 16, 2021, at 09:34, vignesh C wrote:
> On Sat, Jan 16, 2021 at 1:40 AM Andres Freund wrote:
> >
> > On 2021-01-15 09:53:05 +0100, Peter Eisentraut wrote:
> > > On 2020-12-08 10:38, vignesh C wrote:
> > > > I have implemented printing of backtrace based on handling it in
> > > >
On Sat, Jan 16, 2021 at 1:40 AM Andres Freund wrote:
>
> On 2021-01-15 09:53:05 +0100, Peter Eisentraut wrote:
> > On 2020-12-08 10:38, vignesh C wrote:
> > > I have implemented printing of backtrace based on handling it in
> > > CHECK_FOR_INTERRUPTS. This patch also includes the change to allow
On 2021-01-15 09:53:05 +0100, Peter Eisentraut wrote:
> On 2020-12-08 10:38, vignesh C wrote:
> > I have implemented printing of backtrace based on handling it in
> > CHECK_FOR_INTERRUPTS. This patch also includes the change to allow
> > getting backtrace of any particular process based on the
On 2020-12-08 10:38, vignesh C wrote:
I have implemented printing of backtrace based on handling it in
CHECK_FOR_INTERRUPTS. This patch also includes the change to allow
getting backtrace of any particular process based on the suggestions.
Attached patch has the implementation for the same.
On Tue, Dec 1, 2020 at 2:15 PM vignesh C wrote:
>
> On Tue, Dec 1, 2020 at 9:31 AM Tom Lane wrote:
> >
> > Andres Freund writes:
> > > It should be quite doable to emit such backtraces directly to stderr,
> > > instead of using appendStringInfoString()/elog().
> >
> > No, please no.
> >
> > (1)
On Tue, Dec 1, 2020 at 9:31 AM Tom Lane wrote:
>
> Andres Freund writes:
> > It should be quite doable to emit such backtraces directly to stderr,
> > instead of using appendStringInfoString()/elog().
>
> No, please no.
>
> (1) On lots of logging setups (think syslog), anything that goes to
>
Andres Freund writes:
> It should be quite doable to emit such backtraces directly to stderr,
> instead of using appendStringInfoString()/elog().
No, please no.
(1) On lots of logging setups (think syslog), anything that goes to
stderr is just going to wind up in the bit bucket. I realize that
On Tue, 1 Dec 2020 at 11:31, Andres Freund wrote:
>
> Hi,
>
> On 2020-11-30 13:35:46 +0800, Craig Ringer wrote:
> > I find that when I most often want a backtrace of a running, live
> > backend, it's because the backend is doing something that isn't
> > passing a CHECK_FOR_INTERRUPTS() so it's
Hi,
On 2020-11-30 13:35:46 +0800, Craig Ringer wrote:
> I find that when I most often want a backtrace of a running, live
> backend, it's because the backend is doing something that isn't
> passing a CHECK_FOR_INTERRUPTS() so it's not responding to signals. So
> it wouldn't help if a backend is
Hi,
On 2020-11-22 01:25:08 -0500, Tom Lane wrote:
> Surely this is *utterly* unsafe. You can't do that sort of stuff in
> a signal handler.
That's of course true for the current implementation - but I don't think
it's a fundamental constraint. With a bit of care backtrace() and
On Tue, 1 Dec 2020 at 07:04, Tom Lane wrote:
> I'd feel better about it if the mechanism had you specify exactly
> one target process, and were restricted to a superuser requestor.
Er, rather. I actually assumed the former was the case already, not
having looked closely yet.
Craig Ringer writes:
>> I would like to see some discussion of the security implications
>> of such a feature, as well. ("There aren't any" is the wrong
>> answer.)
> If the stack only goes to the log, I actually don't think there are
> significant security implications beyond those we already
> Surely this is *utterly* unsafe. You can't do that sort of stuff in
> a signal handler.
Not safely, anyway. The signal handler could be called in the middle
of a malloc(), a pfree(), or all sorts of other exciting
circumstances. It'd have to be extremely careful to use only local
resources on
On Sun, Nov 22, 2020 at 11:55 AM Tom Lane wrote:
>
> vignesh C writes:
> > The idea here is to implement & expose pg_print_callstack function,
> > internally what this function does is, the connected backend will send
> > SIGUSR1 signal by setting PMSIGNAL_BACKTRACE_EMIT to the postmaster
> >
vignesh C writes:
> The idea here is to implement & expose pg_print_callstack function,
> internally what this function does is, the connected backend will send
> SIGUSR1 signal by setting PMSIGNAL_BACKTRACE_EMIT to the postmaster
> process. Postmaster process will send a SIGUSR1 signal to the
Hi,
I would like to propose getting the callstack of the postgres process
by connecting to the server. This helps us in diagnosing the problems
from a customer environment in case of hung process or in case of long
running process.
The idea here is to implement & expose pg_print_callstack
101 - 135 of 135 matches
Mail list logo
