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

Queueing requestors with an external api #34

Merged
merged 1 commit into from Oct 6, 2014
Merged

Conversation

oferrigni
Copy link
Collaborator

Take member queuing:

There is a new options to configure a pool:
queue_max: Maximum depth of the requestor queue before returning error_no_members

Semantics are as follows
pooler:take_member/2 makes a blocking request to pooler. The second argument is a timeout after which, the pool will reply with error_no_members. If queue_max has been reached, error_no_members will be returned immediately.
take_member/1 is now an alias to take_member_queued/2 with the timeout set to zero

take_member_from_pool_queued(Pool0 = #pool{queue_max = QMax, queued_requestors = Requestors}, CPid, From, Timeout) ->
case {take_member_from_pool(Pool0, CPid), queue:len(Requestors)} of
{{error_no_members, Pool1}, QMax} ->
{error_no_members, Pool1};
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

metrics or logging

@oferrigni
Copy link
Collaborator Author

@seth In a better spot for review. Bookkeeping extracted to helpers, new metrics.

@oferrigni oferrigni changed the title WIP Swag at queueing requestors with an external api Queueing requestors with an external api Oct 5, 2014
@seth
Copy link
Collaborator

seth commented Oct 6, 2014

I think I found the dialyzer issue. Fixes are on sf/queueing branch. I mixed in a bunch of whitespace and formatting changes and some variable name refactor. That was messy of me :(

The core of the dialyzer issue was that take_member_from_pool was spec'd to only allow {pid(), _} which it then passed to take_member_bookkeeping (new name) which actually allowed bare pids. I changed the code to avoid the bare pid special case.

%% @doc Obtain exclusive access to a member from 'PoolName'.
%%
%% If no free members are available, reply is deferred for up to
%% 5 seconds. During this window, if accept member is called
Copy link

Choose a reason for hiding this comment

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

Docs say 5 seconds, but timeout is passed in as an arg and I can't find where it defaults to 5. Is this just an old comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Doc is old, will fix. Timeout is specified by the consumer. A timeout of zero requires a member to be immediately free.

gen_server:call(PoolName, take_member, infinity).
gen_server:call(PoolName, {take_member, 0}, infinity).

%% @doc Obtain exclusive access to a member from 'PoolName'.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"member of"?

@seth
Copy link
Collaborator

seth commented Oct 6, 2014

👍

Take_member now supports request queueing! This feature allows for consumers to
specify a timeout during which a pool member may be returned to a caller. A
timeout of zero means that a member or error will be immediately returned.
Otherwise, take_member will defer replying to the consumer, setting a timer
to the duration of the timeout. When the timer pops, error_no_members will be
sent to the consumer. If during the timeout a new member is returned to the
pool, the requestor queue will be popped and that requestor will receive the
the member.
oferrigni added a commit that referenced this pull request Oct 6, 2014
Queueing requestors with an external api
@oferrigni oferrigni merged commit d0b0e6b into master Oct 6, 2014
@oferrigni oferrigni deleted the of/queueing branch October 6, 2014 23:50
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.

None yet

3 participants