Fix gtp_gtp monitors #432
Fix gtp_gtp monitors #432
Conversation
Currently, a PID is not monitored if it is present in both the contexts and monitors maps. However, there are cases where a monitor is created in the monitors list in a register call and then moved to the contexts list in a bind call, so a new monitor on the same process is created but not tracked. After a time these monitors fill up the memory and crash the node. This commit will not add a new monitor if the PID is present either in the contexts or the monitors maps. It seems this behaviour causes traffic problems, so it is not a final fix.
…r_monitor Module pgw_s5s8_proxy has debug messages for all related traffic functions. The gtp_path:register_monitor function now handles cases where a Pid is present in the contexts or monitors sections separately, although the current implementation is currently the same.
|
I'm afraid it is the logging that enabled us to see the issue. I really miss a lot of debug logs, specially in the handle_* functions for each gen_server instance. We can work out the current format, which I agree is really hasty and not too pretty, but the logs must remain if we want to solve issues like these in the future. We can hold a meeting during the week to work out the details. So far we haven't seen any degradation in memory or speed due to the logs, so that's another reason to keep them in. But as I said, we can discuss the specific format. My plan is to add finer grained tracing later on, so specific IPs or IMSIs can be traced with minimal impact on production nodes (although this would still have to be checked by QA). Currently we're enabling specific modules for logging, but we should be able to further filter by e.g. function according to https://erlang.org/doc/apps/kernel/logger_chapter.html#filters |
|
Logging on a full loaded PGW is useless, not only drown the relevant log entries in the noise of the not relevant messages, it also overloads the system when activated. if you need gen_server/gen_statem event logging, you can always use 50ed2c2 |
|
Logger can have a very fine control about what traces are produced. I don't want to add a new layer on top of something that already works. We can discuss the format, but the traces on handle_X should remain. |
|
@RoadRunnr @javiermtorres: if the only thing that hangs around in the air is log-messages / log-lines: the PR here is against the 2.8er branch and not really relevant for further maybe-it-has-impact situations. so, for now it would be fine with me to keep the log-lines on debug-level in. whats more crucial: the fix for the memory-situation caused by the monitors. so, either way: please merge against 2.8er branch so i can have a 2.8.15 and thus continue in some of our other products. |
The branch fix/gtp_path_monitors contains experimental changes to address memory consumption.
This PR is mainly to have a discussion about the proposed changes