-
Notifications
You must be signed in to change notification settings - Fork 23
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
Users supervision tree #175
Conversation
d2554fa
to
bc98fd8
Compare
433ef34
to
bd0690a
Compare
bc98fd8
to
9c4e2ce
Compare
bd0690a
to
56da56b
Compare
1aa771e
to
9df2ea3
Compare
b663b82
to
58e461c
Compare
In the original implementation, the process is monitored by two supervisors, the original amoc_users_sup, and the amoc_controller one. But this means that bursts of ups and downs overflow the mailbox of _two_ processes, one of them being actually the very critical controller. With this reimplementation, only the amoc_users_sup tracks the processes, and all requests from the controller are asynchronous. This ensures the controller does not get blocked and can remain responsive to control requests.
Useful if for example we want to spawn a big pool of users all at once. Optimised by not setting any timers, but simply looping new user requests.
If users go up and down too fast, amoc_users_sup becomes a bottleneck as it can’t keep up with the requests. Improve the performance by creating a pool of supervisors whose size is proportional to the number of schedulers, and ensure distribution of users among them by for example hashing on the user_id. We do this by creating a pool of supervisors and having the whole API through `amoc_users_sup_sup` choose supervisors according to requests.
Have the user supervisors loop over the users start instead of the controller.
9578920
to
d775663
Compare
d775663
to
30e7646
Compare
30e7646
to
037dc05
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #175 +/- ##
==========================================
+ Coverage 73.44% 75.00% +1.55%
==========================================
Files 29 31 +2
Lines 1043 1160 +117
==========================================
+ Hits 766 870 +104
- Misses 277 290 +13 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this new implementation sufferrs from the same issues as the existing one. removal operation is async and if we call amoc_controller:remove_users/2
two times in a row, it may select the same users for removal. so it won't remove the expected amount of users (the number returned by amoc_controller:remove_users/2
). I don't think that we need to worry about it, but we must to add a note in the documentation about this behaviour.
src/users/amoc_user.erl
Outdated
stop(Pid, false) when is_pid(Pid), Pid =/= self() -> | ||
proc_lib:stop(Pid, shutdown, ?SHUTDOWN_TIMEOUT); | ||
stop(Pid, true) when is_pid(Pid), Pid =/= self() -> | ||
proc_lib:stop(Pid, kill, ?SHUTDOWN_TIMEOUT). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this implementation is not acceptable, we should never kill the user without giving it a chance to finalize its jobs. this interface must be based on some amoc_users_worker_sup
API. The code must be unified, SHUTDOWN_TIMEOUT must be defined at amoc_users_worker_sup
only.
7effd57
to
29d9ea2
Compare
b55455c
to
8ad85c5
Compare
c74f00f
to
11ffbba
Compare
f9c95c4
to
2d16cf7
Compare
2d16cf7
to
623a89f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the huge amount of work, it looks really greate :)
Dedup controller and users sup #172
In the original implementation, the process is monitored by two supervisors, the original amoc_users_sup, and the amoc_controller one. But this means that bursts of ups and downs overflow the mailbox of two processes, one of them being actually the very critical controller.
With this reimplementation, only the amoc_users_sup tracks the processes, and all requests from the controller are asynchronous. This ensures the controller does not get blocked and can remain responsive to control requests.
Interarrival of zero #173
Useful if for example we want to spawn a big pool of users all at once. Optimised by not setting any timers, but simply looping new user requests.
Pool user supervision trees
If users go up and down too fast, amoc_users_sup becomes a bottleneck as it
can’t keep up with the requests. Improve the performance by creating a pool of
supervisors whose size is proportional to the number of schedulers, and ensure
distribution of users among them by for example hashing on the user_id.
We do this by creating a pool of
amoc_users_worker_sup
supervisors and having the whole API throughamoc_users_sup
choose supervisors according to requests.