Implement send_request as in gen_server #187
Conversation
An important change is actually how the queue manager does the call: the client would send a call to the queue manager with the whole request, and the queue manager would then do a cast_call to the worker with the whole request again, incurring in two copies of the request on each erlang message send, which, for very big payloads, can mean lots of copying. Now, the client, within the queue manager's API, calls the queue manager for an available worker, and when it gets it, calls the worker with the whole payload, doing the copies only once. This also uncovers crashes that could happen for other strategies, that wouldn't happen for the available_worker one as it was doing a very wide catch.
This takes the already consumed time away from the next timeout.
Since OTP23, `gen_server` exposes some kind of "asynchronous calls", that is, a mechanism to make a call, and do other stuff while we wait for the result. `send_request` returns a reference, that can be used to match any incoming answers later on, either using `gen_server:wait_for_response/2`, or to do `gen_server:check_response/2` for any message received in a `receive` statement or a `handle_info` clause.
Codecov Report
@@ Coverage Diff @@
## main #187 +/- ##
==========================================
- Coverage 92.82% 92.09% -0.73%
==========================================
Files 10 10
Lines 432 443 +11
==========================================
+ Hits 401 408 +7
- Misses 31 35 +4
Continue to review full report at Codecov.
|
| %% @equiv gen_server:cast(Process, {call, From, Call}) | ||
| -spec cast_call(wpool:name() | pid(), from(), term()) -> ok. | ||
| cast_call(Process, From, Call) -> | ||
| gen_server:cast(Process, {call, From, Call}). |
There was a problem hiding this comment.
WOW! I didn't realize we could already remove this super-annoying function. Amazing!
src/wpool_queue_manager.erl
Outdated
| {ok, TimeLeft, Worker} -> | ||
| case TimeLeft > 0 of |
There was a problem hiding this comment.
| {ok, TimeLeft, Worker} -> | |
| case TimeLeft > 0 of | |
| {ok, TimeLeft, Worker} when TimeLeft > 0 -> |
Stylistic change, ag, fixed, thanks for noticing! The expiration timeouts, there's basically two steps going on: finding the available worker, and once we have it, calling it. Those are two conceptually separated steps, but the second will have less time available for itself: the amount of time the first had already consumed, therefore the substraction. Then we just need to check, after the substraction, if we still have available time. Before, the whole request was run on the context of the queue manager. Now, the client asks the queue manager for a worker, and then the client sends the request himself to the worker, timeouts fixed. |
|
Considering #98 … I would merge this even with the coverage reduction. |
Since OTP23,
gen_serverexposes some kind of "asynchronous calls", that is, a mechanism to make a call, and do other stuff while we wait for the result.send_requestreturns a reference, that can be used to match any incoming answers later on, either usinggen_server:wait_for_response/2, or to dogen_server:check_response/2for any message received in areceivestatement or ahandle_infoclause.See some commit messages for details.