Date: Fri, 13 Sep 2024 07:56:21 -0700
From: Noah Misch <noah@leadboat.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>,
	Jacob Champion <jacob.champion@enterprisedb.com>,
	Andrew Dunstan <andrew@dunslane.net>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
	Euler Taveira <euler.taveira@enterprisedb.com>
Subject: Re: [PATCH] pg_stat_activity: make slow/hanging authentication more
 visible
Message-ID: <20240913145621.43.nmisch@google.com>
References: <20240630174812.d1.nmisch@google.com>
 <CAOYmi+=1Y4HKATADR=WYOMhxCkRQpoxH2rY0t1An6A5su1Cw-w@mail.gmail.com>
 <34cc0828-d988-4503-9944-ea6288f0ca8a@dunslane.net>
 <ZtUCcpf9y7gnHifu@paquier.xyz>
 <CAOYmi+n4RhhoaduZ-ndZGR_=kkzUjnaZtzi8g2GFsqt==Hhopg@mail.gmail.com>
 <Zt_ZVcW4NmPM9NRB@paquier.xyz>
 <20240910172712.e8.nmisch@google.com>
 <CA+Tgmoade_mRdZQHMub31Y9YKg5FpZzwvzYP35R8AVszoRGWcg@mail.gmail.com>
 <20240910205850.5d.nmisch@google.com>
 <CA+Tgmoa9t-ei=mNjyjdcXxQOY=r0PO-+ncHpryjLaFgV9Y0tyg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CA+Tgmoa9t-ei=mNjyjdcXxQOY=r0PO-+ncHpryjLaFgV9Y0tyg@mail.gmail.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: <https://www.postgresql.org/message-id/20240913145621.43.nmisch%40google.com>
Precedence: bulk

On Wed, Sep 11, 2024 at 09:00:33AM -0400, Robert Haas wrote:
> On Tue, Sep 10, 2024 at 4:58 PM Noah Misch <noah@leadboat.com> wrote:
> > ... a rule of "each wait event appears in one
> > pgstat_report_wait_start()" would be a rule I don't want.
> 
> As the original committer of the wait event stuff, I intended for the
> rule that you do not want to be the actual rule. However, I see that I
> didn't spell that out anywhere in the commit message, or the commit
> itself.
> 
> > I see this level of fine-grained naming
> > as making the event name a sort of stable proxy for FILE:LINE.  I'd value
> > exposing such a proxy, all else being equal, but I don't think wait event
> > names like AuthLdapBindLdapbinddn/AuthLdapBindUser are the right way.  Wait
> > event names should be more independent of today's code-level details.
> 
> I don't agree with that. One of the most difficult parts of supporting
> PostgreSQL, in my experience, is that it's often very difficult to
> find out what has gone wrong when a system starts behaving badly. It
> is often necessary to ask customers to install a debugger and do stuff
> with it, or give them an instrumented build, in order to determine the
> root cause of a problem that in some cases is not even particularly
> complicated. While needing to refer to specific source code details
> may not be a common experience for the typical end user, it is
> extremely common for me. This problem commonly arises with error
> messages

That is a problem.  Half the time, error verbosity doesn't disambiguate enough
for me, and I need backtrace_functions.  I now find it hard to believe how
long we coped without backtrace_functions.

I withdraw the objection to "each wait event appears in one
pgstat_report_wait_start()".