Add hibernate_after listener opt with 0 default value. #1821
Conversation
kzemek
force-pushed
the
exml-rapidxml-hibernate
branch
2 times, most recently
from
8a18db6 to
bfd32e4
Compare
kzemek
changed the title
kzemek
force-pushed
the
exml-rapidxml-hibernate
branch
from
bfd32e4 to
5f4a249
Compare
Codecov Report
@@ Coverage Diff @@
## master #1821 +/- ##
==========================================
+ Coverage 74.67% 74.69% +0.01%
==========================================
Files 298 298
Lines 27764 27784 +20
==========================================
+ Hits 20734 20752 +18
- Misses 7030 7032 +2
Continue to review full report at Codecov.
|
|
Do we exchange a lot of presences in our tests? |
|
@arcusfelis These are presence-based load tests: without hibernate and with hibernate. |
|
And here are load tests with users sending messages rapidly: without hibernate and with hibernate. |
|
one_to_one test is more about message sending. Presence based should have:
The roster size also plays a role. I am fine with the change, but still it would be nice to have presence-oriented tests as part of our testing on tide. We don't wake up processes often in one_to_one test. |
|
@arcusfelis Have you looked at the scenarios tested? The tests I linked as "presence tests" only test sending presence updates. Each user has 8-user roster and updates presence every time when in a normal test he would send a message to his 8 neighbors. In the "rapid send" test, each user sends 5 messages/second and receives 40 messages/second. |
kzemek
force-pushed
the
exml-rapidxml-hibernate
branch
from
5f4a249 to
b9d5113
Compare
src/ejabberd_receiver.erl
Outdated
| @@ -421,3 +423,15 @@ gen_server_call_or_noproc(Pid, Message) -> | |||
| end. | |||
|
|
|||
| gen_fsm() -> p1_fsm. | |||
|
|
|||
| hibernate(Result) -> | |||
| case process_info(self(), message_queue_len) of | |||
There was a problem hiding this comment.
With p1_fsm some messages can be inside an internal message queue.
This means that you should check '$internal_queue_len' process dictionary entry too.
I would rather put hibernate logic inside p1_fsm_old.
There was a problem hiding this comment.
👍 for moving the logic to p1_fsm_old.
src/ejabberd_receiver.erl
Outdated
| @@ -421,3 +423,15 @@ gen_server_call_or_noproc(Pid, Message) -> | |||
| end. | |||
|
|
|||
| gen_fsm() -> p1_fsm. | |||
|
|
|||
| hibernate(Result) -> | |||
There was a problem hiding this comment.
Both hibernate/1 and maybe_hibernate/1 needs specs.
I am not fan of element/2 call to get state, because it's not dialyzer verifiable.
Just case can be a better option.
|
So, if we send a message each minute, than in the old test we never hibernate at all => can lead to higher memory usage. |
kzemek
force-pushed
the
exml-rapidxml-hibernate
branch
3 times, most recently
from
31c2942 to
8ca7050
Compare
kzemek
force-pushed
the
exml-rapidxml-hibernate
branch
from
8ca7050 to
8346606
Compare
The default of 0 means that the processes hibernate on every opportunity.
kzemek
force-pushed
the
exml-rapidxml-hibernate
branch
from
8346606 to
2a1db2a
Compare
This PR introduces a new configuration variable for listeners:
hibernate_after.This variable configures the timeout (in milliseconds) after which a c2s and receiver process will hibernate if they're idle. It replaces old in-code constants that hibernated these processes after 90 seconds of idle time.
If
hibernate_afteris 0 (the default), the processes will hibernate as soon as they get a chance. Load tests have shown that this greatly reduces MongooseIM's memory consumption while producing no noticeable slowdown or increase in CPU load under common scenarios.