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

Report waiting time only for currently waiting clients #678

Merged
merged 4 commits into from
Jan 18, 2024

Conversation

drdrsh
Copy link
Collaborator

@drdrsh drdrsh commented Jan 17, 2024

The pool maxwait metric currently operates differently from Pgbouncer.

The way it operates today is that we keep track of max_wait on each connected client, when SHOW POOLS query is made, we go over the connected clients and we get the max of max_wait times among clients. This means the pool maxwait will never reset, it will always be monotonically increasing until the client with the highest maxwait disconnects.

This PR changes this behavior, by keeping track of the wait_start time on each client, when a client goes into WAITING state, we record the time offset from connect_time. When we either successfully or unsuccessfully checkout a connection from the pool, we reset the wait_start time.

When SHOW POOLS query is made, we go over all connected clients and we only consider clients whose wait_start is non-zero, for clients that have non-zero wait times, we compare them and report the maximum waiting time as maxwait for the pool.

@drdrsh drdrsh marked this pull request as ready for review January 18, 2024 02:47
@drdrsh drdrsh requested a review from levkk January 18, 2024 02:47
@drdrsh drdrsh changed the title Report waiting time only for currently witing clients Report waiting time only for currently waiting clients Jan 18, 2024
@@ -41,6 +41,11 @@ pub struct ClientStats {
/// Maximum time spent waiting for a connection from pool, measures in microseconds
pub max_wait_time: Arc<AtomicU64>,

// Time when the client started waiting for a connection from pool, measures in microseconds
// We use connect_time as the reference point for this value
// U64 can represent ~60 centuries in microseconds, so we should be fine
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is more like 5850 centuries, I got my math wrong.

@drdrsh drdrsh merged commit e1e4929 into main Jan 18, 2024
1 check passed
@levkk levkk deleted the mostafa_fix_timing branch January 18, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant