Skip to content

Commit

Permalink
fix(gproc_pool): fix rand:uniform/1 range (#193)
Browse files Browse the repository at this point in the history
Since
[`rand:uniform/1`](https://www.erlang.org/doc/man/rand.html#uniform-1)'s
return value is a number in a closed interval (`0 =< N =< Sz`), we
shouldn't call it with the pool size + 1, otherwise we will get a
non-uniform distribution of results where one of the workers receive
substantially more work than all others.

To verify this, I've started a pool with 8 workers using the `random`
load-balancing algorithm and measured the frequencies of each PID in
`gproc_pool`.

```erlang
PoolName = my_pool.
PoolSize = 8.
ok = gproc_pool:new(PoolName, random, [{size, PoolSize}]).
lists:foreach(fun(I) ->
  gproc_pool:add_worker(PoolName, {PoolName, I}, I),
  spawn(fun() ->
    true = gproc_pool:connect_worker(PoolName, {PoolName, I}),
    receive
        stop -> ok
    end
  end),
  ok
end, lists:seq(1, PoolSize)).
MeasureFreqs = fun(Pool, NumPoints) ->
  Pids = lists:map(fun(_) -> gproc_pool:pick_worker(Pool) end, lists:seq(1, NumPoints)),
  lists:foldl(
    fun(Pid, Acc) ->
      maps:update_with(Pid, fun(N) -> N + 1 end, 1, Acc)
    end,
    #{},
    Pids
   )
end.
MeasureFreqs(PoolName, 100_000).
```

Results prior to the fix (note how `<0.172.0>` has almost double the
frequency of other PIDs):

```erlang
 #{<0.172.0> => 22222,<0.173.0> => 11138,<0.174.0> => 11000,
  <0.175.0> => 11046,<0.176.0> => 11141,<0.177.0> => 11165,
  <0.178.0> => 11166,<0.179.0> => 11122}
```

After the fix:

```erlang
 #{<0.172.0> => 12460,<0.173.0> => 12367,<0.174.0> => 12541,
  <0.175.0> => 12494,<0.176.0> => 12649,<0.177.0> => 12399,
  <0.178.0> => 12439,<0.179.0> => 12651}
```
  • Loading branch information
thalesmg authored May 4, 2023
1 parent 3856e76 commit 4ca45e0
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/gproc_pool.erl
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ pick(Pool, Sz, round_robin, Ret) ->
end
end;
pick(Pool, Sz, random, Ret) ->
pick_near(Pool, rand:uniform(Sz + 1), Ret).
pick_near(Pool, rand:uniform(Sz), Ret).

pick(Pool, Sz, hash, Val, Ret) ->
pick_near(Pool, erlang:phash2(Val, Sz) + 1, Ret);
Expand Down Expand Up @@ -606,7 +606,7 @@ randomize(Pool) ->
0 -> 0;
1 -> 1;
Sz ->
incr(Pool, rand:uniform(Sz + 1) - 1, Sz)
incr(Pool, rand:uniform(Sz) - 1, Sz)
end.

%% @spec pool_size(Pool::any()) -> integer()
Expand Down

0 comments on commit 4ca45e0

Please sign in to comment.