Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ $ sudo net-with-gdb

The `net-loadtest` launchs a temporary db and a nginx server, waiting until a number of requests are done, then reporting results (plus process monitoring) at the end.

It takes a two parameters: the number of requests (GETs) and the `pg_net.batch_size`.
It takes up to four optional parameters: the number of requests (GETs), the `pg_net.batch_size`, and a server delay in ms with its standard deviation in ms (normally distributed). Run `net-loadtest --help` for details.

```bash
$ net-loadtest 1000 200
Expand Down
31 changes: 31 additions & 0 deletions nix/loadtest.nix
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,31 @@ in
writeShellScriptBin "net-loadtest" ''
set -euo pipefail

usage() {
cat <<EOF
Usage: net-loadtest [REQUESTS] [BATCH_SIZE] [DELAY_MS] [DELAY_SD_MS]

Launch a temporary db and nginx server, wait until REQUESTS GETs are done,
then report results plus process monitoring (CPU/MEM) as markdown.

Arguments (all optional, positional):
NUM_REQUESTS number of GET requests (default: 10000)
BATCH_SIZE value for pg_net.batch_size (default: extension default)
DELAY_MS mean server delay in ms, normally distributed (default: none)
DELAY_SD_MS standard deviation of the server delay in ms (default: 0)

Options:
-h, --help show this help and exit

Example:
net-loadtest 1000 200 50 10
EOF
}

case "''${1:-}" in
-h|--help) usage; exit 0 ;;
esac

reqs=""
batch_size_opt=""

Expand All @@ -88,6 +113,12 @@ writeShellScriptBin "net-loadtest" ''
batch_size_opt="-c pg_net.batch_size=$2"
fi

# Optional server delay in ms following a normal distribution.
Comment thread
mkaruza marked this conversation as resolved.
# Arguments: delay in ms ($3), delay standard deviation in ms ($4)
if [ -n "''${3:-}" ]; then
reqs="$reqs, delay_ms => ''${3}, delay_sd_ms => ''${4:-0}"
fi

net-with-nginx xpg --options "-c log_min_messages=WARNING $batch_size_opt" \
psql -c "call wait_for_many_gets($reqs)" -c "\pset format csv" -c "\o $query_csv" -c "select * from run" > /dev/null &

Expand Down
11 changes: 3 additions & 8 deletions nix/nginx/conf/custom.conf
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,9 @@ location / {
echo 'Hello world';
}

location /slow-reply {
echo_sleep 2;
echo 'this text will come in response body with HTTP 200 after 2 seconds';
}

location /really-slow-reply {
echo_sleep 30;
echo 'this text will come in response body with HTTP 200 after 30 seconds';
location /delay {
echo_sleep $arg_s;
echo 'ok';
}

location /echo-method {
Expand Down
29 changes: 27 additions & 2 deletions test/utils/loadtest.sql
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,29 @@ create table run (
last_failure_error text
);

-- random delay in ms from a normal distribution with the given mean and standard deviation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk about everyone else but I'm not a huge fan of putting performance testing scripts into CI before merge. I feel like CI should be only for hard failures: testing internal logic, thread safety issues, memory safety stuff, etc. Maybe I'm totally off base here but just wondering what everyone else thinks.

@mkaruza mkaruza Oct 17, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR doesn't enable or add load test that will run in CI with this feature. But I share same view that for CI performance/benchmark should not be running.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I was not criticising this PR in particular, just using it as an opportunity to open discussion.

I do think performance testing is important and should be done, just not sure when/how. Maybe on every commit on merge so u can look at the exact commit a regression occurred? Idk

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk about everyone else but I'm not a huge fan of putting performance testing scripts into CI before merge

I'm not sure about this PR, but continuous benchmarking has been very helpful to ensure correctness. At some point we got random timeouts during big batches and CI detects those (request failures at 0) plus it shows that CPU usage should be low. The runs are pretty stable too (see CI results, all the batches are completed in < 1 min and that has been maintained)

It's yet incomplete though, it should fail when time_taken and CPU usage surpasses a threshold.

@steve-chavez steve-chavez Oct 20, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked the loadtest and there's a regression #321, this proves it's useful and should keep running on PRs IMO.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A per-commit performance regression suite is indeed valuable. What we also need are graphs that show performance over time which is easier to spot visually than visit each commit's results as Steve did above. Also a little more sophisticated statistical analysis will help gauge the trends better as currently we only have raw numbers.

Ideally we run the benchmarks on non-GH runners to avoid noisy-neighbour problems. I ran the CI multiple times on the master branch and there is a noise of at least 2%, this run's max CPU was 13%, while this one was at 11%. I also suspect there's a regression but it becomes harder to compare these small changes on GH runners. Maybe have dedicated machines for benchmarking?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The noise is worse than 2%. Another example of two runs on the same commit:

  • This run had the max CPU at around 7% with one outlier at 9%.
  • This run had the max CPU at around 12% with a couple of them on 13%.

This has a variation of 5% points in max CPU, which is not great to compare performance. Also we should be comparing the mean and standard deviation to meaningfully compare two runs, but with the current runs this is what we have.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest that we move CI discussion and tradeoffs to some other channel so we have clear picture what would be acceptable behavior.

-- Uses random_normal(), which is only available on PostgreSQL 16+.
-- Negative results are clamped to 0 since a delay can't be negative.
create or replace function random_delay_ms(mean_ms int, sd_ms int) returns int as $$
begin
if current_setting('server_version_num')::int < 160000 then
raise exception 'function `random_normal` requires PostgreSQL 16 or newer, running %', current_setting('server_version');
end if;
return greatest(0, round(random_normal(mean_ms, sd_ms)))::int;
end;
$$ language plpgsql volatile;

-- loadtest using many gets, used to be called `repro_timeouts`
create or replace procedure wait_for_many_gets(number_of_requests int default 10000, url text default 'http://localhost:8080') as $$
--
-- when delay_ms is given, each request goes to the nginx /delay endpoint with a server delay drawn
-- from a normal distribution with mean delay_ms and standard deviation delay_sd_ms (clamped
-- at 0), to simulate the latency of real servers
create or replace procedure wait_for_many_gets(
number_of_requests int default 10000,
url text default 'http://localhost:8080',
delay_ms int default null,
delay_sd_ms int default 0
) as $$
declare
last_id bigint;
first_time timestamptz;
Expand All @@ -22,7 +43,11 @@ begin

with do_requests as (
select
net.http_get(url) as id
net.http_get(
case when delay_ms is null then url
else url || '/delay?s=' || round(random_delay_ms(delay_ms, delay_sd_ms) / 1000.0, 3)
Comment thread
imor marked this conversation as resolved.
end
) as id
from generate_series (1, number_of_requests) x
)
select id, clock_timestamp() into last_id, first_time from do_requests offset number_of_requests - 1;
Expand Down
Loading