#### Pre-Submission Checklist - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [x] Each component has a single commit (if not, squash them into one commit) - [x] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: - [ ] PR should be backported to stable branches - [x] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description This PR moves the selected hash up the call stack so it is more accessible in failover, and by the higher order functions.
It is a little hard to see the benefit until I get the next few patchsets submitted, but I will link them below this description. The biggest benefit for other dispatcher algorithm developers is that the hash selected is now available on failover and can be used to lookup attrs (or any other info) needed for implementing more complex failure scenarios.
These are the examples I am working on that depend on this change (as an example of how it can be used): 1. [rpc commands][1] - an example where a user can test selection algorithms with a dataset, without stepping into gdb 2. [failover selection][2] - an example algorithm that makes use of this change for re-balancing the distribution on failover (in mode 1)
[1]: https://github.com/devopsec/kamailio/tree/dispatcher_rpc_updates [2]: https://github.com/devopsec/kamailio/tree/dispatcher_alg_updates You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/4500
-- Commit Summary --
* dispatcher: make hash result more accessible
-- File Changes --
M src/modules/dispatcher/dispatch.c (201) M src/modules/dispatcher/dispatch.h (16) M src/modules/dispatcher/dispatcher.c (148)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/4500.patch https://github.com/kamailio/kamailio/pull/4500.diff
@devopsec pushed 1 commit.
98a83c9c1717af49a5aca8538dc49e9bfbd0cebb dispatcher: make hash result more accessible
devopsec left a comment (kamailio/kamailio#4500)
fixed formatting with clang-format
miconda left a comment (kamailio/kamailio#4500)
I would prefer not to return structures from functions, they can be inefficient if their size is big, which tends to happen over the time. It is also not the common practice we have in the code so far.
If you need the value of the hash for failover, wouldn't it be better to keep it in the context xavp?
- https://www.kamailio.org/docs/modules/stable/modules/dispatcher.html#dispatc...
devopsec left a comment (kamailio/kamailio#4500)
Hmmm, ds_xavp_dst works accessing it on failover but the other use case (the one the ds_hres_t was created for) is accessing it on the first selected destination, even when failover is not turned on. Is there a better way to grab the original ds_dest_t selected in that case?
The example would be the 1st branch i linked above, showing the user the order of destinations when using a specific algorithm with their current data, via an rpc command. I did not see another simple way to pull that selected hash up the call stack. Here is an example (WIP) feature where I used the returned strcture: https://github.com/kamailio/kamailio/compare/master...devopsec:kamailio:disp...
I'll highlight that use case at a higher level for more context. Lets say that I have a customer call in and say they were expected their traffic to be distributed in a different manner than currently is (lets say they have some indication they are not getting the level of service or features they expect). Lets say I have a NOC staffed with technicians that needs to review that customers routing configuration or need to check if changing their routing configuration (dispatcher data / algorithm) would/is giving them the intended distribution. They could test the proposed changes (or current route configuration) using the rpc command in that branch I linked to check where the requests will be distributed without needing to try replicating the live traffic that led to the customer complaint.
The struct made that selected destination available, even when failover was not enabled or the algorithm did not use failover. I am of course willing to refactor to meet that requirement but I am unsure how I could accomplish that use case without returning the dlist[] index selected as well.
miconda left a comment (kamailio/kamailio#4500)
I am going to merge this one, but then change to return again int from functions and have the res structure (ds_hres_t) as output parameter, to match the common coding practice we have so far.
Merged #4500 into master.
miconda left a comment (kamailio/kamailio#4500)
Check to see if the updated varian suits your needs and works as expected -- see the last commit referenced above. If there are issues then, open a bug in the tracker.
devopsec left a comment (kamailio/kamailio#4500)
Understood, looks like that will still accomplish the same task, allowing hash selected to be used when calling ds_select_dst_limit() if needed. Thanks for looking through this @miconda
-
Daniel-Constantin Mierla -
Tyler Moore