<!-- Kamailio Pull Request Template -->
<!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception -->
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [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: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [ ] PR should be backported to stable branches - [X] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description <!-- Describe your changes in detail -->
Added a publish function to nats module. When the function is called, the routing process tries to connect to the url (param) and tries to send the payload (param) to the subject queue (param). No libuv is used for this.
Probably a better approach is to make publisher workers (like subscriber workers) and pass payload from routing processes to publisher worker processes via shared memory. Right now I see this as an improvement and find the new function good enough, for now.
@eschmidbauer what do you think of this?
Thank you, Stefan
P.S. Will update doc after review. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/2915
-- Commit Summary --
* <a href="https://github.com/kamailio/kamailio/pull/2915/commits/121ae374e0c309027eebeabcff39031dc316d297">nats: Add publish function</a>
-- File Changes --
M src/modules/nats/nats_mod.c (71)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2915.patch https://github.com/kamailio/kamailio/pull/2915.diff
@eschmidbauer commented on this pull request.
it looks like this code will connect to NATS server each publish request. I think it would be better to handle the connections outside of the `nats_publish_f` function to avoid having to connect each request.
@smititelu pushed 1 commit.
e736af98038663cbbb2abf0a3c16c42a7aea8bea nats: Add publish function
Updated PR to connect to urls while parsing the given "nats_url" modparams. Also the new function now supports either 1, 2 or 3 params; url and subject are now optional; it will publish to all urls and all subject queues if none given.
Basically tested it with 3 local nats servers. And calling the new config function either with 1 param or 3 params.
Let me know what you think of this.
Thank you, Stefan
@smititelu pushed 1 commit.
601932d8981a268f02bb878376d188b14c22c323 nats: Add publish function
force-pushed with doc update :D
Thanks, Stefan
@smititelu pushed 1 commit.
78b1fa4c8d764f6774bd949c176c3394e018e8cf nats: Add publish function
re-updated PR to add callbacks to publisher connections; right now the publisher callbacks are the same as the subscriber callbacks.
Thus, kamailio nats module can make use of nats lib reconnection(e.g. natsOptions_SetRetryOnFailedConnect() function) and not implement a reconnect mechanism by itself.
I will later make a new PR to make the parameters that control nats lib reconnect mechanism, configurable: ``` natsOptions_SetMaxReconnect(opts, 10000); // how many times nats lib tries to reconnect before giving up; -1 is forever if i'm not wrong natsOptions_SetReconnectWait(opts, 2 * 1000); // the interval between reconnect retries ```
Thanks, Stefan
@smititelu this looks good, nice work and thank you! I would like to pull & test this branch if you don't mind waiting a few days (sorry I'm super swamped with work)
@eschmidbauer commented on this pull request.
@@ -519,6 +614,7 @@ int init_nats_server_url_add(char *url)
n = n->next; } n = _init_nats_server_list_new(url); + _init_nats_server_conn(n);
this initiates connections to the servers but each subscriber still maintains a separate connection to each server. i think it would make sense to change all subscriber workers to use the same connections as publishers
@smititelu I made some changes to better handle the NATS connections and share the connections between publishes and subscribers. This eliminates duplicate connection handling within the module and allows it to extend into independent connection handling (e.g. add/remove/label nats connections while kamailio is running) later. I've submitted the PR to your fork [here](https://github.com/smititelu/kamailio/pull/2/files) so you can review and test. It's not a common implementation to specify a nats url per publish request therefore the message publish will be sent to a single nats connection (which can container up to 10 nats urls). Also, I did not see the benefit of mixing queue group subjects with publish requests, therefore, I thought it best to also specify subject on publish request. I think there is more to be done here but mostly around testing and fixup of publish parameters. Please let me know your thoughts, and if you don't mind, let's move this conversation to your fork so we can discuss best path forward before submitting the code to kamailio repo.
Closing this, because #2978 has a better approach, using publisher workers.
Closed #2915.
-
Emmanuel Schmidbauer -
Stefan Mititelu