Skip to content
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

rtpengine: refactor of node probing #2597

Merged
merged 3 commits into from
Aug 13, 2021

Conversation

john08burke
Copy link
Contributor

This PR is a slight refactor of the existing probing mechanism for disabled rtpengine nodes. Currently, probing is done within the SIP processing context. This can lead to excessive delay while processing SIP requests when 1+ nodes are down or unreachable. For example, theoretical max delay in SIP context is (# down nodes) * (rtpengine_retr * rtpengine_tout) seconds every rtpengine_disable_tout second interval when the down nodes are selected via weighting algorithm.

This PR proposes to move the probing mechanism out of SIP context and into a timer routine that triggers every rtpengine_timer_interval second interval. This should eliminate delays associated with probing disabled rtpengine nodes.

Let me know your thoughts! The same could likely be applied to the rtpproxy module, but I wanted to get feedback before refactoring rtpproxy.

Probing of disabled rtpengine nodes is now done in timer routine instead of SIP context.
Copy link
Member

@razvancrainea razvancrainea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good, and I indeed agree this new approach is better than the initial one.
Could you please address the change I suggested? If you do, I think we're good to merge it.

modules/rtpengine/rtpengine.c Outdated Show resolved Hide resolved
@razvancrainea razvancrainea merged commit 15321ba into OpenSIPS:master Aug 13, 2021
@razvancrainea
Copy link
Member

Thanks for the PR, John! The rtpproxy one is welcome as well, if you have time to take care of it :).

@john08burke
Copy link
Contributor Author

Hey @razvancrainea, I will try to put a PR in for the rtpproxy module. I was first going to look into an additional PR for rtpengine, where rtpengine_reload mi command wouldn't grab global lock on the node list. When nodes are down, it has similar downsides that this PR addressed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants