Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Durations reported by SHOW STATS are in milliseconds, not microseconds #810

Open
smcgivern opened this issue Sep 9, 2024 · 3 comments
Open
Labels
bug Something isn't working

Comments

@smcgivern
Copy link
Contributor

Describe the bug

PgBouncer reports *_time metrics in SHOW STATS in microseconds. PgCat reports the same metrics in milliseconds.

https://github.com/postgresml/pgcat/tree/main#statistics-reporting says:

The stats are very similar to what PgBouncer reports and the names are kept to be comparable

So I'd expect the units to be the same too.

PgBouncer source here: https://github.com/pgbouncer/pgbouncer/blob/e2a2a682fe9069366fca970ec2492d774b4f6b40/src/server.c#L556-L559

To Reproduce

Steps to reproduce the behavior:

  1. Run Postgres and connect both PgBouncer and PgCat.
  2. Run SELECT pg_sleep(1); via both connection poolers.
  3. Run SHOW STATS; in both connection poolers.

PgBouncer will give something like:

total_xact_count  | 1
total_query_count | 1
total_received    | 25
total_sent        | 107
total_xact_time   | 1007496
total_query_time  | 1007496
total_wait_time   | 3499
avg_xact_count    | 0
avg_query_count   | 0
avg_recv          | 0
avg_sent          | 3
avg_xact_time     | 1007496
avg_query_time    | 1007496
avg_wait_time     | 121

Whereas PgCat says:

total_xact_count  | 1
total_query_count | 1
total_received    | 107
total_sent        | 62
total_xact_time   | 0
total_query_time  | 1001
total_wait_time   | 7
total_errors      | 0
avg_xact_count    | 0
avg_query_count   | 0
avg_recv          | 7
avg_sent          | 4
avg_errors        | 0
avg_xact_time     | 0
avg_query_time    | 1001
avg_wait_time     | 0

(xact_time metrics are already reported missing in #114.)

@smcgivern
Copy link
Contributor Author

This is not true for all stats, wait_time seems to still be in microseconds:

pgcat/src/pool.rs

Lines 789 to 793 in b9ec7f8

let checkout_time = now.elapsed().as_micros() as u64;
client_stats.checkout_success();
server
.stats()
.checkout_time(checkout_time, client_stats.application_name());

@drdrsh drdrsh added the bug Something isn't working label Sep 10, 2024
@drdrsh
Copy link
Collaborator

drdrsh commented Sep 10, 2024

Good find.
I think we should fix this but it is a bit of breaking change so it should be noted in the release notes.

@smcgivern
Copy link
Contributor Author

Should we just change this for the stats reported by the PgBouncer-compatible interface?

I'm guessing people using the Prometheus metrics endpoint would already be happy with the current units. Either way, we should probably mention the units in the help text there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants