Skip to content

Commit

Permalink
chore: remove worker state from handle cancelled callback
Browse files Browse the repository at this point in the history
  • Loading branch information
oliveigah committed Mar 21, 2024
1 parent f6dbcee commit 15923e4
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 26 deletions.
24 changes: 11 additions & 13 deletions lib/nimble_pool.ex
Original file line number Diff line number Diff line change
Expand Up @@ -305,19 +305,17 @@ defmodule NimblePool do
The context argument may be `:queued` or `:checked_out`:
* `:queued` means the cancellation happened before resource checkout. This may happen
when the pool is starving under load and can not serve resources. Since no checkout
happened the worker_state argument will be `nil`.
when the pool is starving under load and can not serve resources.
* `:checked_out` means the cancellation happened after resource checkout. This may happen
when the function given to `checkout!/4` raises.
This callback is optional.
"""
@doc callback: :worker
@doc callback: :pool
@callback handle_cancelled(
worker_state :: worker_state | nil,
pool_state,
context :: :queued | :checked_out
context :: :queued | :checked_out,
pool_state
) :: :ok

@optional_callbacks init_pool: 1,
Expand All @@ -328,7 +326,7 @@ defmodule NimblePool do
handle_ping: 2,
terminate_worker: 3,
terminate_pool: 2,
handle_cancelled: 3
handle_cancelled: 2

@doc """
Defines a pool to be started under the supervision tree.
Expand Down Expand Up @@ -777,17 +775,17 @@ defmodule NimblePool do
case requests do
# Exited or timed out before we could serve it
%{^ref => {_, mon_ref, :command, _command, _deadline}} ->
if function_exported?(worker, :handle_cancelled, 3) do
args = [nil, pool_state, :queued]
if function_exported?(worker, :handle_cancelled, 2) do
args = [:queued, pool_state]
apply_worker_callback(worker, :handle_cancelled, args)
end

{:noreply, remove_request(state, ref, mon_ref)}

# Exited or errored during client processing
%{^ref => {_, mon_ref, :state, worker_server_state}} ->
if function_exported?(worker, :handle_cancelled, 3) do
args = [worker_server_state, pool_state, :checked_out]
if function_exported?(worker, :handle_cancelled, 2) do
args = [:checked_out, pool_state]
apply_worker_callback(worker, :handle_cancelled, args)
end

Expand All @@ -796,8 +794,8 @@ defmodule NimblePool do

# The client timed out, sent us a message, and we dropped the deadlined request
%{} ->
if function_exported?(worker, :handle_cancelled, 3) do
args = [nil, pool_state, :queued]
if function_exported?(worker, :handle_cancelled, 2) do
args = [:queued, pool_state]
apply_worker_callback(worker, :handle_cancelled, args)
end

Expand Down
26 changes: 13 additions & 13 deletions test/nimble_pool_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ defmodule NimblePoolTest do
TestAgent.next(pool_state.state, :terminate_pool, [reason, pool_state])
end

def handle_cancelled(worker_state, pool_state, context) do
TestAgent.next(pool_state, :handle_cancelled, [worker_state, pool_state, context])
def handle_cancelled(context, pool_state) do
TestAgent.next(pool_state, :handle_cancelled, [context, pool_state])
end
end

Expand Down Expand Up @@ -1023,7 +1023,7 @@ defmodule NimblePoolTest do
handle_checkout: fn :checkout, _from, _next, pool_state ->
{:ok, :client_state_out, :server_state_out, pool_state}
end,
handle_cancelled: fn :server_state_out, _pool_state, :checked_out -> :ok end,
handle_cancelled: fn :checked_out, _pool_state -> :ok end,
terminate_worker: fn reason, :server_state_out, state ->
send(parent, {:terminate, reason})
{:ok, state}
Expand Down Expand Up @@ -1059,7 +1059,7 @@ defmodule NimblePoolTest do
handle_update: fn :update, _next, pool_state ->
{:ok, :updated_state, pool_state}
end,
handle_cancelled: fn :server_state_out, _pool_state, :checked_out -> :ok end,
handle_cancelled: fn :checked_out, _pool_state -> :ok end,
terminate_worker: fn reason, :updated_state, pool_state ->
send(parent, {:terminate, reason})
{:ok, pool_state}
Expand Down Expand Up @@ -1698,8 +1698,8 @@ defmodule NimblePoolTest do
handle_checkout: fn :checkout, _from, worker_state, pool_state ->
{:ok, :client_state_out, worker_state, pool_state}
end,
handle_cancelled: fn worker_state, _pool_state, :checked_out ->
send(parent, {:ping, worker_state})
handle_cancelled: fn :checked_out, _pool_state ->
send(parent, :ping)
:ok
end,
terminate_worker: fn _reason, _, state -> {:ok, state} end
Expand All @@ -1716,7 +1716,7 @@ defmodule NimblePoolTest do
end
)

assert_receive({:ping, :worker1})
assert_receive(:ping)

NimblePool.stop(pool, :shutdown)
end
Expand All @@ -1731,8 +1731,8 @@ defmodule NimblePoolTest do
handle_checkout: fn :checkout, _from, worker_state, pool_state ->
{:ok, :client_state_out, worker_state, pool_state}
end,
handle_cancelled: fn nil, _pool_state, :queued ->
send(parent, {:ping, nil})
handle_cancelled: fn :queued, _pool_state ->
send(parent, :ping)
:ok
end,
handle_checkin: fn :client_state_in, _from, next, pool_state ->
Expand Down Expand Up @@ -1766,7 +1766,7 @@ defmodule NimblePoolTest do

send(task1.pid, :release)

assert_receive({:ping, nil})
assert_receive(:ping)

NimblePool.stop(pool, :shutdown)
end
Expand All @@ -1778,8 +1778,8 @@ defmodule NimblePoolTest do
stateful_pool!(
[
init_worker: fn next -> {:ok, :worker1, next} end,
handle_cancelled: fn nil, _pool_state, :queued ->
send(parent, {:ping, nil})
handle_cancelled: fn :queued, _pool_state ->
send(parent, :ping)
:ok
end,
handle_checkout: fn :checkout, _from, worker_state, pool_state ->
Expand Down Expand Up @@ -1807,7 +1807,7 @@ defmodule NimblePoolTest do

:sys.resume(pool)

assert_receive({:ping, nil})
assert_receive(:ping)

assert NimblePool.checkout!(pool, :checkout, fn _ref, :client_state_out ->
{:result, :client_state_in}
Expand Down

0 comments on commit 15923e4

Please sign in to comment.