Horton

Re: [PATCH] OAuth: fix performance bug with stuck multiplexer events

Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>

View thread
From: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Thomas Munro <thomas.munro@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Daniel Gustafsson <daniel@yesql.se>, Peter Eisentraut <peter@eisentraut.org>
Date: 2025-08-07T16:35:29Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. oauth: Always link with -lm for floor()

  2. oauth: Add unit tests for multiplexer handling

  3. oauth: Ensure unused socket registrations are removed

  4. oauth: Remove expired timers from the multiplexer

  5. oauth: Remove stale events from the kqueue multiplexer

  6. oauth: Track total call count during a client flow

Jacob Champion <jacob.champion@enterprisedb.com> writes:

> From 50257bf32eb2b0972e5139ac4a79367372c77385 Mon Sep 17 00:00:00 2001
> From: Jacob Champion <jacob.champion@enterprisedb.com>
> Date: Wed, 5 Mar 2025 15:04:34 -0800
> Subject: [PATCH v3 5/5] oauth: Add unit tests for multiplexer handling

I haven't read the meat of the patch, but I have some comments on the
tests:

> +IPC::Run::run ['oauth_tests'],
> +  '>', IPC::Run::new_chunker, sub { print {$out} $_[0] },
> +  '2>', IPC::Run::new_chunker, sub { print {$err} $_[0] }
> +  or die "oauth_tests returned $?";

We've recently switched to using fat commas (=>) between options and
their arguments, and that includes the file redirections in IPC::Run.
Although not semantically meaningful, I'd also be tempted to put parens
around the argument list for each redirect, so it's clear that they go
together.

Also, indirect object syntax (print {$fh} ...) is ugly and
old-fashioned, it's nicer to call it as a method on the filehandle.

So I'd write the above as:

    IPC::Run::run ['oauth_tests'],
      '>' => (IPC::Run::new_chunker, sub { $out->print($_[0]) }),
      '2>' => (IPC::Run::new_chunker, sub { $err->print($_[0]) })
     or die "oauth_tests returned $?";


As for the C TAP tests, there's already a bunch of TAP-outputting
infrastructure in pg_regress.c.  Would it make sense to factor that out
into a common library?

- ilmari