Skip to content

Conversation

@steve-chavez
Copy link

@steve-chavez steve-chavez commented Jan 28, 2022

Hello @avanov ,

As mentioned in bos#42 (comment), I'd like to limit the waiter queue to do backpressure. So I've added totalWaiters to PoolStats.

Related: PostgREST/postgrest#2042 (comment)

LMWYT, thanks!

@avanov
Copy link
Owner

avanov commented Feb 3, 2022

This looks good and useful semantically. What I'm not sure of is whether the length of the queue should be calculated here or inside WaiterQueue module. Logically it's a property of the queue entity that could be read from it, but never modified directly.

@steve-chavez
Copy link
Author

@avanov Thanks for the review. I did it like this because I also plan to add maxWaiters and I think that wouldn't fit inside the WaiterQueue module. Seemed more consistent to have them both outside.

@steve-chavez steve-chavez marked this pull request as draft February 8, 2022 14:55
@avanov
Copy link
Owner

avanov commented Feb 8, 2022

@steve-chavez yeah I get that from the linked discussion, though aren't they two different concepts? One has to do with a "peekable" stat value (property of the queue), another has to do with utilising all available underlying stats for higher level interface of back-pressuring (back-pressuring being a property of the queue). For instance, the following sequence:

removeSelf <- push waiters var
modifyTVar_ waitersSizeVar (+ 1)

it assumes that push always leads to (+1) for the stat value, and that pop results in (subtract 1).

@steve-chavez
Copy link
Author

@avanov Sorry for the late reply here. Since the newest hasql-pool doesn't use resource-pool(ref), I no longer need this change(we're transitioning to the newest hasql-pool PostgREST/postgrest#2391).

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.

2 participants