Thread
Commits
-
oauth: Skip call-count test for libcurl 8.20.0
- c5c35fd7c55c 18 (unreleased) landed
- fd5ea2e9e37c 19 (unreleased) landed
-
libpq-oauth: Print libcurl version with OAUTHDEBUG_UNSAFE_TRACE
- 357e4d64f871 18 (unreleased) landed
- 4bd477dcc619 19 (unreleased) landed
-
[oauth] Increased CPU usage during device flow with libcurl 8.20.0
Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-15T19:28:17Z
Hi all, A couple of macOS testers pinged me last week with a newly failing test in oauth_validator/001_server: not ok 113 - call count is reasonably small It's a heuristic test, so I was looking into whether that heuristic needed to be tweaked. But this is actually a legitimate failure, caused by an upstream regression in Curl 8.20.0 [1]. I've tested the most recent 8.21.0 RC, due to release later this month, and the test now passes again. So that just leaves what to do about the current test failures. My current idea is to just skip the test if the curl binary reports that specific minor version. (A configure test probably won't help very much after this month: anyone consuming rolling releases of Curl in production will have already built against a past version, and if an upcoming LTS distro chose 8.20.0 as its base, they'd almost certainly backport the fix too, making the check worse than nothing.) An alternative would be to just let it ride for a couple weeks, but I don't really want to inflict that on our Homebrew testers. Either way, debugging this has inspired some improvements to OAUTHDEBUG_UNSAFE_TRACE that I'll propose after the 19 freeze is over. Thoughts? --Jacob [1] https://github.com/curl/curl/issues/21547 -
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-15T19:47:42Z
Jacob Champion <jacob.champion@enterprisedb.com> writes: > So that just leaves what to do about the current test failures. My > current idea is to just skip the test if the curl binary reports that > specific minor version. Seems like a good solution. We don't know how long such curl binaries will persist in the wild. regards, tom lane
-
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-15T22:56:52Z
On Mon, Jun 15, 2026 at 12:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Jacob Champion <jacob.champion@enterprisedb.com> writes: > > So that just leaves what to do about the current test failures. My > > current idea is to just skip the test if the curl binary reports that > > specific minor version. > > Seems like a good solution. We don't know how long such curl > binaries will persist in the wild. Cool. Unfortunately, I immediately ran into an obvious-in-retrospect problem: the Homebrew libcurl we're linked against is not what you see when you type `curl --version` at the command line, and I don't think we record the runtime version of libcurl anywhere today. I've attached a solution that should work well for PG20, but I don't feel as good about it for 19 (or a backport to 18). I need to switch to a different context for today, but tomorrow I'll try to find a solution that touches only the test code. Thanks, --Jacob
-
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-16T19:46:31Z
On Mon, Jun 15, 2026 at 3:56 PM Jacob Champion <jacob.champion@enterprisedb.com> wrote: > I need to switch to a different context for today, but tomorrow I'll > try to find a solution that touches only the test code. Attached as v2-0002, which moves the version check into one of the OAuth test executables. (I'll hold 0004 until after REL_19_STABLE is branched; it just implements the v1 strategy and reverts 0002.) I've tested this against a local Homebrew installation, but if anyone who's hit this in the wild has a chance to put 0001-3 through a smoke test, that'd be awesome. Barring any objections or bad test results, I'll plan to push tomorrow. Thanks! --Jacob
-
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Daniel Gustafsson <daniel@yesql.se> — 2026-06-16T20:11:53Z
> On 16 Jun 2026, at 21:46, Jacob Champion <jacob.champion@enterprisedb.com> wrote: > Attached as v2-0002, which moves the version check into one of the > OAuth test executables. (I'll hold 0004 until after REL_19_STABLE is > branched; it just implements the v1 strategy and reverts 0002.) +#if USE_LIBCURL + +/* + * XXX You may wonder why this test executable, which purposely does not make + * use of libcurl functionality, is printing out the version of Curl. This is + * needed to skip tests in 001_server when we see a known broken version of + * libcurl. (Querying the local Curl executable isn't good enough, because that + * may not use the same libcurl that we've been configured with.) If there is concern over the neatness of this (long term in the backbranches, for master it will be reverted as mentioned), I guess one option could be to introduce a new binary, oauth_curl_version, which only prints the version and exits instead oa adding a flag to an otherwise unrelated binary. It would add a little compilation overhead though. -- Daniel Gustafsson
-
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-16T20:40:50Z
Jacob Champion <jacob.champion@enterprisedb.com> writes: > Attached as v2-0002, which moves the version check into one of the > OAuth test executables. (I'll hold 0004 until after REL_19_STABLE is > branched; it just implements the v1 strategy and reverts 0002.) > I've tested this against a local Homebrew installation, but if anyone > who's hit this in the wild has a chance to put 0001-3 through a smoke > test, that'd be awesome. Barring any objections or bad test results, > I'll plan to push tomorrow. I confirm that, with or without 0004, this fixes the oauth_validator failure on the machine where I saw that. However ... I don't love the plan of fixing this differently in v19 and v20 just because of feature freeze. Exposing more information for testing purposes isn't a user-visible feature IMO, so I would rather we go straight to 0004. CC'ing the RMT to see if they agree. (I think the rmt@ alias is not functioning, so cc'ing members directly.) regards, tom lane
-
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-16T20:58:04Z
On Tue, Jun 16, 2026 at 1:12 PM Daniel Gustafsson <daniel@yesql.se> wrote: > If there is concern over the neatness of this (long term in the backbranches, > for master it will be reverted as mentioned), I guess one option could be to > introduce a new binary, oauth_curl_version, which only prints the version and > exits instead oa adding a flag to an otherwise unrelated binary. It would add > a little compilation overhead though. I considered that, but I think adding a second binary to the existing Makefile is likely to result in build system churn (to get it to behave like e.g. test_json_parser/Makefile). I can investigate that path if you think it'd be more maintainable that way, but let's make sure Tom's point below doesn't moot it: On Tue, Jun 16, 2026 at 1:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I confirm that, with or without 0004, this fixes the oauth_validator > failure on the machine where I saw that. Thanks very much! > However ... I don't love the plan of fixing this differently in v19 > and v20 just because of feature freeze. Exposing more information > for testing purposes isn't a user-visible feature IMO, so I would > rather we go straight to 0004. Fair enough. If the RMT is okay with this for 19, were you thinking we'd also backpatch that code directly to 18? --Jacob
-
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-16T21:02:28Z
Jacob Champion <jacob.champion@enterprisedb.com> writes: > On Tue, Jun 16, 2026 at 1:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> However ... I don't love the plan of fixing this differently in v19 >> and v20 just because of feature freeze. Exposing more information >> for testing purposes isn't a user-visible feature IMO, so I would >> rather we go straight to 0004. > Fair enough. If the RMT is okay with this for 19, were you thinking > we'd also backpatch that code directly to 18? Sure. regards, tom lane
-
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Nathan Bossart <nathandbossart@gmail.com> — 2026-06-17T15:40:04Z
[RMT hat] On Tue, Jun 16, 2026 at 05:02:28PM -0400, Tom Lane wrote: > Jacob Champion <jacob.champion@enterprisedb.com> writes: >> On Tue, Jun 16, 2026 at 1:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> However ... I don't love the plan of fixing this differently in v19 >>> and v20 just because of feature freeze. Exposing more information >>> for testing purposes isn't a user-visible feature IMO, so I would >>> rather we go straight to 0004. > >> Fair enough. If the RMT is okay with this for 19, were you thinking >> we'd also backpatch that code directly to 18? > > Sure. This seems like a reasonable plan to me. -- nathan
-
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Melanie Plageman <melanieplageman@gmail.com> — 2026-06-17T15:50:57Z
On Wed, Jun 17, 2026 at 11:40 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > > >> Fair enough. If the RMT is okay with this for 19, were you thinking > >> we'd also backpatch that code directly to 18? > > > > Sure. > > This seems like a reasonable plan to me. +1
-
Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0
Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-17T20:57:05Z
On Wed, Jun 17, 2026 at 8:51 AM Melanie Plageman <melanieplageman@gmail.com> wrote: > On Wed, Jun 17, 2026 at 11:40 AM Nathan Bossart > <nathandbossart@gmail.com> wrote: > > > This seems like a reasonable plan to me. > > +1 Great, thanks everyone! I've pushed a patchset based on v1, after some additional testing. --Jacob