3 Nov
2021
3 Nov
'21
9:35 a.m.
<!-- 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/121ae374e…quot;>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
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915
3 Nov
3 Nov
6:15 p.m.
@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.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915#pullrequestreview-797157946
4 Nov
4 Nov
8:21 a.m.
@smititelu pushed 1 commit.
e736af98038663cbbb2abf0a3c16c42a7aea8bea nats: Add publish function
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2915/files/121ae374e0c309027eebea…
8:25 a.m.
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
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915#issuecomment-960798522
10:24 a.m.
@smititelu pushed 1 commit.
601932d8981a268f02bb878376d188b14c22c323 nats: Add publish function
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2915/files/e736af98038663cbbb2abf…
10:25 a.m.
force-pushed with doc update :D
Thanks,
Stefan
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915#issuecomment-961045655
5 Nov
5 Nov
9:07 a.m.
@smititelu pushed 1 commit.
78b1fa4c8d764f6774bd949c176c3394e018e8cf nats: Add publish function
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2915/files/601932d8981a268f02bb87…
9:15 a.m.
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
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915#issuecomment-961885703
2:34 p.m.
@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)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915#issuecomment-962125752
14 Nov
14 Nov
6:54 a.m.
@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
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915#pullrequestreview-805462498
8:48 a.m.
@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.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915#issuecomment-968294247
22 Dec
22 Dec
2:58 a.m.
Closing this, because #2978 has a better approach, using publisher workers.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915#issuecomment-999362044
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/2915/c999362044(a)github.com>
2:58 a.m.
Closed #2915.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2915#event-5803181382
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/2915/issue_event/5803181382(a)github.com>
908
days inactive
957
days old
12 comments
2 participants
participants (2)
-
Emmanuel Schmidbauer -
Stefan Mititelu