On Wed, May 21, 2025 at 5:20 PM Jacob Champion
wrote:
>
> I took a quick look at the authentication and oauth_validator changes.
> Maybe 007_pre_auth.pl should include `receipt`, to make it easier to
> debug pre-auth hangs using the logs? Other than that, the changes
> looked reasonable to me.
Co
On 20.05.25 17:16, Melanie Plageman wrote:
In src/backend/tcop/postgres.c, there is a call
SetConfigOption("log_connections", "true", context, source);
that could be adjusted.
Do you think the debug option should be 'all' or a list of the options
covered by "true" (whic
On 21.05.25 21:53, Melanie Plageman wrote:
On Tue, May 20, 2025 at 11:16 AM Melanie Plageman
wrote:
In earlier versions of my patch, I played around with replacing these references in the docs. I ended up not doing it because I wasn't sure we had
consensus on deprecating the "on", "true", "ye
On Wed, May 21, 2025 at 12:54 PM Melanie Plageman
wrote:
> Attached is a patch that updates these as well as changes all usages
> of log_connections in the tests. I made some judgment calls about
> where we might want expanded or reduced log_connections aspects. As
> such, the patch could use a on
On Tue, May 20, 2025 at 11:16 AM Melanie Plageman
wrote:
>
> In earlier versions of my patch, I played around with replacing these
> references in the docs. I ended up not doing it because I wasn't sure we had
> consensus on deprecating the "on", "true", "yes" options and that we would
> contin
On Tue, May 20, 2025 at 4:52 AM Peter Eisentraut
wrote:
> log_connections has been changed from a Boolean parameter to a string
> one, but a number of places in the documentation and in various pieces
> of test code still use the old values. I think it would be better if
> they were adjusted to
> On 20 May 2025, at 10:52, Peter Eisentraut wrote:
> src/test/ssl/t/SSL/Server.pm
While I don't have an immediate usecase or test in mind, having the SSL tests
use log_conections=all could be handy when hacking on the TLS support.
--
Daniel Gustafsson
On 12.03.25 16:43, Melanie Plageman wrote:
On Tue, Mar 11, 2025 at 6:27 PM Melanie Plageman
wrote:
I did more manual testing of my patches, and I think they are mostly
ready for commit except for the IsConnectionBackend() macro (if we
have something to change it to).
I've committed this and
On Tue, Mar 11, 2025 at 6:27 PM Melanie Plageman
wrote:
>
> I did more manual testing of my patches, and I think they are mostly
> ready for commit except for the IsConnectionBackend() macro (if we
> have something to change it to).
I've committed this and marked it as such in the CF app.
Thanks
On Mon, Mar 10, 2025 at 5:03 PM Daniel Gustafsson wrote:
>
> > On 7 Mar 2025, at 23:08, Melanie Plageman wrote:
>
> Sorry for being late to the party. I like this functionality and would
> definitely like to see it in v18.
Thanks so much for the review!
> + /* For backwards compatibility, we
Hi,
On Fri, Mar 07, 2025 at 03:29:14PM -0500, Melanie Plageman wrote:
> On Fri, Mar 7, 2025 at 6:16 AM Bertrand Drouvot
> wrote:
> > But at the end, what we're "really" interested in this thread, given its
> > $SUBJECT,
> > is to actually log the timings.
>
> I'm not sure that people would enab
> On 7 Mar 2025, at 23:08, Melanie Plageman wrote:
Sorry for being late to the party. I like this functionality and would
definitely like to see it in v18.
> An unrelated note about the attached v13:
> I changed the language from log_connections "stages" to "options" and
> "aspects" depending o
On Thu, Mar 6, 2025 at 3:16 PM Melanie Plageman
wrote:
>
> Jacob at some point had asked about this, and I didn't
> have a satisfactory answer. I'm not really sure what would be more
> useful to end users.
For the record, I'm not really sure either. I don't have a strong
opinion either way.
--Ja
On Fri, Mar 7, 2025 at 10:09 AM Bertrand Drouvot
wrote:
>
> Given that conn_timing.ready_for_use is only used here:
>
> + if (!reported_first_ready_for_query &&
> + (log_connections &
> LOG_CONNECTION_READY_FOR_USE) &&
> +
On Fri, Mar 7, 2025 at 2:17 PM Melanie Plageman
wrote:
> Yea, "auth_id" is too ambiguous for a GUC option. I went with
> "authorization" because it sounds a bit less like a stage while also
> being a recognizable word. It achieves symmetry -- though it doesn't
> match the prefix used in the logs.
On Fri, Mar 7, 2025 at 4:53 PM Jacob Champion
wrote:
>
> If I add a hypothetical auth method in the future that authenticates,
> and then farms the authorization decision out to some slow-moving
> network machinery, would "authenticated" retroactively become a stage
> then? (OAuth almost does this
Thanks for the review!
On Fri, Mar 7, 2025 at 2:08 AM Fujii Masao wrote:
>
> With the patch, any unambiguous prefix of a valid boolean value,
> like 'y', is no longer accepted even though it's currently valid
> for boolean GUCs. I don’t have a strong opinion on whether
> we should maintain compat
On Fri, Mar 7, 2025 at 12:29 PM Melanie Plageman
wrote:
> This got me thinking more about the existing log connections messages
> and whether or not they are actually all "stages". The authenticated
> and authorized messages occur at nearly the same time in the
> connection establishment timeline.
On Fri, Mar 7, 2025 at 6:16 AM Bertrand Drouvot
wrote:
>
> On Thu, Mar 06, 2025 at 11:41:03AM -0500, Melanie Plageman wrote:
>
> > Well, I actually didn't want to call it "timings" because it implies
> > that we only measure the timings when it is specified. But we actually
> > get the timings unc
Hi,
On Thu, Mar 06, 2025 at 06:16:07PM -0500, Melanie Plageman wrote:
> Attached v12 does this (uses timestamps instead of instr_time).
Thanks for the new version!
> I've done some assorted cleanup. Most notably:
>
> I removed LOG_CONNECTION_NONE because it could lead to wrong results
> to have
Hi,
On Thu, Mar 06, 2025 at 11:41:03AM -0500, Melanie Plageman wrote:
>
> I still need to do some more manual testing and validation.
>
> On Thu, Mar 6, 2025 at 9:56 AM Bertrand Drouvot
> wrote:
>
> > + /* If an empty string was passed, we're done. */
> > + if (list_length(elemlist
Hi,
On Thu, Mar 06, 2025 at 02:10:47PM -0500, Andres Freund wrote:
> On 2025-01-20 15:01:38 +, Bertrand Drouvot wrote:
> > /* If start_time is in the future or now, no time has elapsed */
> > if (start_time >= stop_time)
> > return 0;
> > "
> >
> > I think that it can happen d
On 2025/03/07 8:16, Melanie Plageman wrote:
On Thu, Mar 6, 2025 at 2:10 PM Andres Freund wrote:
I think it'd be better to use absolute times and store them as such in
ConnectionTimes or whatever. That way we have information about when a
connection was established for some future SQL functi
On Thu, Mar 6, 2025 at 2:10 PM Andres Freund wrote:
>
> I think it'd be better to use absolute times and store them as such in
> ConnectionTimes or whatever. That way we have information about when a
> connection was established for some future SQL functions and for debugging
> problems.
Attached
Hi,
On 2025-01-20 15:01:38 +, Bertrand Drouvot wrote:
> Regarding the TimestampTz vs instr_time choice, we have things like:
>
> + TimestampTz fork_time = ((BackendStartupData *) startup_data)->fork_time;
> + TimestampTz cur_time = GetCurrentTimestamp();
> +
> + conn_timing.fork_duration = Ti
Thanks for the continued review!
Attached v11 has a test added (passes locally but fails in CI, so I
have to fix that).
I still need to do some more manual testing and validation.
On Thu, Mar 6, 2025 at 9:56 AM Bertrand Drouvot
wrote:
>
<-- snip -->
> what do you think about also doing?
>
>
Hi,
On Wed, Mar 05, 2025 at 06:40:10PM -0500, Melanie Plageman wrote:
> On Wed, Mar 5, 2025 at 10:46 AM Melanie Plageman
> wrote:
> >
> > As such, I wonder if my PGC_SET idea is not worth the effort and I
> > should revise the earlier patch version which specified the stages but
> > allow for on,
On Wed, Mar 5, 2025 at 10:46 AM Melanie Plageman
wrote:
>
> As such, I wonder if my PGC_SET idea is not worth the effort and I
> should revise the earlier patch version which specified the stages but
> allow for on, off, true, yes, 1 for backwards compatibility and not
> include disconnections (so
On Wed, Mar 5, 2025 at 5:36 AM Bertrand Drouvot
wrote:
>
> Looking at the enum array:
>
> "
> static const struct config_enum_entry log_connections_options[] = {
> {"off", 0, false},
> {"on", LOG_CONNECTION_BASIC, false},
> {"true", LOG_CONNECTION_BASIC, true},
> {"false", 0, true}
Hi,
On Tue, Mar 04, 2025 at 06:25:42PM -0500, Melanie Plageman wrote:
> Attached v9 implements log_connections as an enum with a new third
> value "timings".
>
> On Mon, Mar 3, 2025 at 11:14 AM Bertrand Drouvot
> wrote:
> >
> >
> > On Fri, Feb 28, 2025 at 05:52:35PM -0500, Melanie Plageman wrote
Hi,
On Tue, Mar 04, 2025 at 05:47:26PM -0500, Melanie Plageman wrote:
> On Mon, Mar 3, 2025 at 9:07 AM Bertrand Drouvot
> wrote:
> >
> > I did not imagine that much ;-) I was just seeing this code being duplicated
> > and just thought about to avoid the duplication. But now that I read your
> >
On Tue, Mar 4, 2025 at 3:25 PM Melanie Plageman
wrote:
>
> I don't
> see any other TAP tests that are testing connections. There are a
> whole bunch of auth tests, I suppose.
I was going to suggest src/test/authentication, yeah. Not perfect, but
better than nothing?
--Jacob
On Mon, Mar 3, 2025 at 11:45 PM Fujii Masao wrote:
>
> When log_connection is empty string in postgresql.conf and I ran
> "psql -d "options=-clog_connections=all"", I got the following log message.
> You can see the total duration in this message is unexpected.
> It looks like this happens because
Thanks so much for your review!
On Mon, Mar 3, 2025 at 12:08 PM Fujii Masao wrote:
>
>
> > +bool
> > +check_log_connections(char **newval, void **extra, GucSource source)
> > +{
> >
> > This function is pretty close to check_debug_io_direct() for example and its
> > overall content, memory manage
Attached v9 implements log_connections as an enum with a new third
value "timings".
On Mon, Mar 3, 2025 at 11:14 AM Bertrand Drouvot
wrote:
>
>
> On Fri, Feb 28, 2025 at 05:52:35PM -0500, Melanie Plageman wrote:
>
> > We have to be really careful about when log_connections is actually
> > set bef
On Mon, Mar 3, 2025 at 9:07 AM Bertrand Drouvot
wrote:
>
> I did not imagine that much ;-) I was just seeing this code being duplicated
> and just thought about to avoid the duplication. But now that I read your
> comments
> above then I think we could just macro-ize the child_type check (as you
On Tue, Mar 4, 2025 at 4:40 AM Bertrand Drouvot
wrote:
>
> On Mon, Mar 03, 2025 at 06:24:59PM -0500, Melanie Plageman wrote:
> >
> > And I think it would be even simpler to do your idea of groups and to
> > have aliases for different combinations of options. And it would let
> > us keep 'on' and '
Hi,
On Mon, Mar 03, 2025 at 06:24:59PM -0500, Melanie Plageman wrote:
> On Mon, Mar 3, 2025 at 11:14 AM Bertrand Drouvot
> wrote:
> >
> >
> > On Fri, Feb 28, 2025 at 05:52:35PM -0500, Melanie Plageman wrote:
> >
> > +bool
> > +check_log_connections(char **newval, void **extra, GucSource source)
>
On 2025/03/01 7:52, Melanie Plageman wrote:
On Fri, Feb 28, 2025 at 12:54 AM Bertrand Drouvot
wrote:
On Thu, Feb 27, 2025 at 05:55:19PM -0500, Melanie Plageman wrote:
It still needs polishing (e.g. I need to figure out where to put the new guc
hook
functions and enums and such)
yeah, I
On Mon, Mar 3, 2025 at 11:14 AM Bertrand Drouvot
wrote:
>
>
> On Fri, Feb 28, 2025 at 05:52:35PM -0500, Melanie Plageman wrote:
>
> +bool
> +check_log_connections(char **newval, void **extra, GucSource source)
> +{
>
> This function is pretty close to check_debug_io_direct() for example and its
>
Hi,
On Fri, Feb 28, 2025 at 05:52:35PM -0500, Melanie Plageman wrote:
> On Fri, Feb 28, 2025 at 12:54 AM Bertrand Drouvot
> wrote:
> >
> > On Thu, Feb 27, 2025 at 05:55:19PM -0500, Melanie Plageman wrote:
> > > It still needs polishing (e.g. I need to figure out where to put the new
> > > guc ho
On 2025/03/04 1:14, Bertrand Drouvot wrote:
=== 2
+/*
+ * Validate the input for the log_connections GUC.
+ */
+bool
+check_log_connections(char **newval, void **extra, GucSource source)
+{
This function is pretty close to check_debug_io_direct() for example and its
overall content, memory m
Hi,
On Fri, Feb 28, 2025 at 05:42:37PM -0500, Melanie Plageman wrote:
> And all of these places we have to be super
> careful that the GUCs have already been read before using
> log_connections, so it seems a bit unsafe to check log_connections
> (the global variable) in a function.
Yeah, that co
On Fri, Feb 28, 2025 at 12:54 AM Bertrand Drouvot
wrote:
>
> On Thu, Feb 27, 2025 at 05:55:19PM -0500, Melanie Plageman wrote:
> > It still needs polishing (e.g. I need to figure out where to put the new
> > guc hook
> > functions and enums and such)
>
> yeah, I wonder if it would make sense to c
On Fri, Feb 28, 2025 at 12:16 AM Bertrand Drouvot
wrote:
>
> Yup but my idea was to put all those line:
>
> "
> if (Log_connections &&
> (child_type == B_BACKEND || child_type == B_WAL_SENDER))
> {
> instr_time fork_time = ((BackendStartupData *)
> startup_data)->fork_tim
Hi,
On Thu, Feb 27, 2025 at 05:55:19PM -0500, Melanie Plageman wrote:
> On Thu, Feb 27, 2025 at 11:30 AM Andres Freund wrote:
> >
> > On 2025-02-27 11:08:04 -0500, Melanie Plageman wrote:
> >
> > > However, since that is a bigger project (with more refactoring, etc),
> > > he suggested that we ch
Hi,
On Thu, Feb 27, 2025 at 11:08:04AM -0500, Melanie Plageman wrote:
> I was just talking to Andres off-list and he mentioned that the volume
> of log_connections messages added in recent releases can really be a
> problem for users. He said ideally we would emit one message which
> consolidated
Hi,
On Thu, Feb 27, 2025 at 11:14:56AM -0500, Andres Freund wrote:
> I don't think the timing overhead is a relevant factor here - compared to the
> fork of a new connection or performing authentication the cost of taking a few
> timestamps is neglegible. A timestamp costs 10s to 100s of cycles, a
On Thu, Feb 27, 2025 at 11:30 AM Andres Freund wrote:
>
> On 2025-02-27 11:08:04 -0500, Melanie Plageman wrote:
>
> > However, since that is a bigger project (with more refactoring, etc),
> > he suggested that we change log_connections to a GUC_LIST
> > (ConfigureNamesString) with options like "no
Hi,
On 2025-02-27 11:08:04 -0500, Melanie Plageman wrote:
> I was just talking to Andres off-list and he mentioned that the volume
> of log_connections messages added in recent releases can really be a
> problem for users.
For some added color: I've seen plenty systems where almost all the log vo
Hi,
On 2025-02-27 06:50:41 +, Bertrand Drouvot wrote:
> On Wed, Feb 26, 2025 at 01:45:39PM -0500, Melanie Plageman wrote:
> > Thanks for the continued review!
> >
> > On Wed, Feb 26, 2025 at 2:41 AM Bertrand Drouvot
> > wrote:
> > >
> > > On Wed, Feb 26, 2025 at 01:46:19PM +0900, Fujii Masao
On Thu, Feb 27, 2025 at 1:50 AM Bertrand Drouvot
wrote:
>
> Agree. IIUC, I think that Fujii-san's idea was to extend log_connections with
> a new option "timing" (i.e move it from ConfigureNamesBool to say
> ConfigureNamesEnum with say on, off and timing?). I think that's a good idea.
>
> I just d
Hi,
On Wed, Feb 26, 2025 at 01:45:39PM -0500, Melanie Plageman wrote:
> Thanks for the continued review!
>
> On Wed, Feb 26, 2025 at 2:41 AM Bertrand Drouvot
> wrote:
> >
> > On Wed, Feb 26, 2025 at 01:46:19PM +0900, Fujii Masao wrote:
> > >
> > > With the current patch, when log_connections is
On Wed, Feb 26, 2025 at 1:45 PM Melanie Plageman
wrote:
>
> Thanks for the continued review!
>
> On Wed, Feb 26, 2025 at 2:41 AM Bertrand Drouvot
> wrote:
>
> > Add a few words in the log_connections GUC doc? (anyway we will have to if
> > Fujii-san idea above about the timing is implemented)
>
>
Thanks for the continued review!
On Wed, Feb 26, 2025 at 2:41 AM Bertrand Drouvot
wrote:
>
> On Wed, Feb 26, 2025 at 01:46:19PM +0900, Fujii Masao wrote:
> >
> > With the current patch, when log_connections is enabled, the connection
> > time is always
> > captured, and which might introduce per
Thanks for the review!
On Tue, Feb 25, 2025 at 11:46 PM Fujii Masao
wrote:
>
> On 2025/02/26 6:36, Melanie Plageman wrote:
> > On Tue, Feb 25, 2025 at 3:23 PM Melanie Plageman
> > wrote:
>
> + /* Capture time Postmaster initiates fork for logging */
> + if (child_type == B_BACKEND)
>
On 26/02/2025 08:41, Bertrand Drouvot wrote:
Hi,
On Wed, Feb 26, 2025 at 01:46:19PM +0900, Fujii Masao wrote:
On 2025/02/26 6:36, Melanie Plageman wrote:
On Tue, Feb 25, 2025 at 3:23 PM Melanie Plageman
wrote:
Thanks for doing this! I have implemented your suggestion in attached v3.
Tha
Hi,
On Wed, Feb 26, 2025 at 01:46:19PM +0900, Fujii Masao wrote:
>
>
> On 2025/02/26 6:36, Melanie Plageman wrote:
> > On Tue, Feb 25, 2025 at 3:23 PM Melanie Plageman
> > wrote:
> > >
> > > Thanks for doing this! I have implemented your suggestion in attached v3.
Thanks for the new patch ver
On 2025/02/26 6:36, Melanie Plageman wrote:
On Tue, Feb 25, 2025 at 3:23 PM Melanie Plageman
wrote:
Thanks for doing this! I have implemented your suggestion in attached v3.
I missed an include in the EXEC_BACKEND not defined case. attached v4
is fixed up.
Thanks for updating the patch!
On Tue, Feb 25, 2025 at 3:23 PM Melanie Plageman
wrote:
>
> Thanks for doing this! I have implemented your suggestion in attached v3.
I missed an include in the EXEC_BACKEND not defined case. attached v4
is fixed up.
- Melanie
From c493f0be7243bfe965d3493f07982ea1072d89b7 Mon Sep 17 00:00:00 200
Thanks for taking a look!
On Mon, Jan 20, 2025 at 12:53 PM Jacob Champion
wrote:
>
> On Mon, Jan 20, 2025 at 7:01 AM Bertrand Drouvot
> wrote:
> > Though time changes are "rare", given the fact that those metrics could
> > provide
> > "inaccurate" measurements during that particular moment (tim
Thanks for taking a look!
On Mon, Jan 20, 2025 at 10:01 AM Bertrand Drouvot
wrote:
>
> The patch needed a rebase due to 34486b6092e. I did it in v2 attached (it's
> a minor rebase due to the AmRegularBackendProcess() introduction in
> miscadmin.h).
>
> v2 could rely on AmRegularBackendProcess()
On Mon, Dec 16, 2024 at 6:26 PM Jelte Fennema-Nio wrote:
>
> Two thoughts:
> 1. Would it make sense to also expose these timings in some pg_stat_xyz view?
So, right now there isn't an obvious place this would fit. There are
dynamic statistics views that are per connection -- like
pg_stat_ssl/gssa
On Mon, Jan 20, 2025 at 7:01 AM Bertrand Drouvot
wrote:
> Though time changes are "rare", given the fact that those metrics could
> provide
> "inaccurate" measurements during that particular moment (time change) then it
> might be worth considering instr_time instead for this particular metric.
Hi,
On Mon, Jan 06, 2025 at 10:08:36PM +0100, Guillaume Lelarge wrote:
> Hello,
>
> Le lun. 16 déc. 2024 à 22:00, Melanie Plageman
> a écrit :
>
> > Hi,
> >
> > Users wishing to debug slow connection establishment have little
> > visibility into which steps take the most time. We don't expose a
Hello,
Le lun. 16 déc. 2024 à 22:00, Melanie Plageman
a écrit :
> Hi,
>
> Users wishing to debug slow connection establishment have little
> visibility into which steps take the most time. We don't expose any
> stats and none of the logging includes durations.
>
> The attached patch logs duratio
On Mon, 16 Dec 2024 at 22:00, Melanie Plageman
wrote:
> Users wishing to debug slow connection establishment have little
> visibility into which steps take the most time. We don't expose any
> stats and none of the logging includes durations.
Two thoughts:
1. Would it make sense to also expose th
67 matches
Mail list logo
