postgresml/pgcat

Durations reported by SHOW STATS are in milliseconds, not microseconds

Opened this issue · 3 comments

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.)

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());

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.

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.