This new module, based on libevent and cURL multi interface, implements non blocking HTTP queries. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/480
-- Commit Summary --
* pv: expose some PV API function * async_http: non-blocking HTTP client module
-- File Changes --
A modules/async_http/Makefile (32) A modules/async_http/README (423) A modules/async_http/async_http.c (326) A modules/async_http/async_http.h (131) A modules/async_http/async_http_mod.c (636) A modules/async_http/doc/Makefile (4) A modules/async_http/doc/async_http.xml (51) A modules/async_http/doc/async_http_admin.xml (437) A modules/async_http/hm_hash.c (186) A modules/async_http/hm_hash.h (134) A modules/async_http/http_multi.c (591) A modules/async_http/http_multi.h (65) M modules/pv/pv.c (9)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/480.patch https://github.com/kamailio/kamailio/pull/480.diff
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480
I would like this to be merged into the CURL module (it's mentioned in the todo). Having multiple modules calling CURL is not a good thing, especially if they use HTTPS (multiple initialisations of OpenSSL is not a good thing). That's also why Hugh added an API to the curl module, so we can build upon one single CURL binding.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-172549865
Having said that I agree that we need async HTTP :-)
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-172550176
I agree on having a single curl based module. I started looking at curl module when this module was almost finished, in its core part, with this purpose but, due to the chronic lack of time, the result was that the job on the async module was stagnating beacuse an integration would have meant a deep rework of the code. So we decided to start releasing the module thinking that the integration with other modules can be always done with the help of our wonderful community :)
On Mon, Jan 18, 2016 at 3:52 PM, Olle E. Johansson <notifications@github.com
wrote:
Having said that I agree that we need async HTTP :-)
— Reply to this email directly or view it on GitHub https://github.com/kamailio/kamailio/pull/480#issuecomment-172550176.
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
I am ok with many modules targeting to offer similar functionality, if they have different approach -- like lcr can be achieved with couple of modules. If there is a conflict with another module, that needs to be documented.
Some remarks:
* using functions from pv module must be done via inter-module api structure. The old method to export via module structure for config functions is prone to issues when prototypes are changed, because the cast will hide that. Many modules export now internal API via a bind structure, en example is to look at sl_load_api() in modules/sl/sl.h and how it is used from other modules, like registrar.
* more like personal opinion, I find the name a bit restrictive for the future, just in case one will want to add some non-async features to it. Maybe it would be better for long term to use something more generic, e.g., httpc, http_client, ...
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-173187418
Thanks for this module - a very useful feature! I have a couple of use cases for curl/async_http which are possibly off topic (major enhancements), I mention them here so that if any redesign/code sharing happens they are taken into account. * Sending asynchronous queries without suspending main transaction - e.g. for sending logging/notifications where result is not important - would run an event_route on response so stats etc. can be updated * Streaming multiple requests to the same destination - Avoid TLS handshake overhead for every request
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-173263621
Thanks for the feedback Daniel. I just pushed 2 commits from Camille to implement pv module api and use it in the new module. I'll also add a note on the documentation about the conflict with the curl module. About the name, http_client could be a good one, the current one having been chosen mainly to highlight the asynchronous processing.
On Wed, Jan 20, 2016 at 1:22 PM, Daniel-Constantin Mierla < notifications@github.com> wrote:
I am ok with many modules targeting to offer similar functionality, if they have different approach -- like lcr can be achieved with couple of modules. If there is a conflict with another module, that needs to be documented.
Some remarks:
using functions from pv module must be done via inter-module api structure. The old method to export via module structure for config functions is prone to issues when prototypes are changed, because the cast will hide that. Many modules export now internal API via a bind structure, en example is to look at sl_load_api() in modules/sl/sl.h and how it is used from other modules, like registrar.
more like personal opinion, I find the name a bit restrictive for the future, just in case one will want to add some non-async features to it. Maybe it would be better for long term to use something more generic, e.g., httpc, http_client, ...
— Reply to this email directly or view it on GitHub https://github.com/kamailio/kamailio/pull/480#issuecomment-173187418.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-173475562
Thanks Hugh for the feedback!
On Wed, Jan 20, 2016 at 5:34 PM, Hugh Waite notifications@github.com wrote:
Thanks for this module - a very useful feature! I have a couple of use cases for curl/async_http which are possibly off topic (major enhancements), I mention them here so that if any redesign/code sharing happens they are taken into account.
- Sending asynchronous queries without suspending main transaction
important
- e.g. for sending logging/notifications where result is not
- would run an event_route on response so stats etc. can be updated
This is a good idea and should be not too hard to implement with the
current architecture, I'll have a look and try to have it implemented in the first release.
- Streaming multiple requests to the same destination
- Avoid TLS handshake overhead for every request
This also is an excellent suggestion, but not so easy to implement
currently :) I'll put in the module's TODO list.
Again, thanks for the feedback.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-173476074
Since both modules are using CURL they need similar names. Since they should not be used at the same time, this should be documented. We can still rename the CURL module before release. A big difference is that the curl module use a connection configuration, much like SQLops.
Stubbornly I still think, from a product management standpoint, that these should be the same module before release. If not, either name them CURL and CURL_ASYNC or HTTP and HTTP_ASYNC
Not being able to use them at the same time is a bug from my point of view. The easiest way to fix this is to move the call to CURL from http_async to CURL, add it to the CURL API and have the Http_Async module use the curl module for talking with libcurl. That way, both modules can be loaded at the same time, without a large merge of the code at this point. A huge win for all Kamailio users!
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-173486511
If both modules should be used at the same time, the most problematic point would be calling `curl_global_init()` several times IMO (although the libcurl documentation states that it must be called "at least" once). However, **async_http** uses the `curl_global_init_mem()` version (it allows curl to use kamailio's memory API instead of the libc one), and if it is called after `curl_global_init()`, it will have no effect. We chose to bind libcurl with the SHM memory API, because the amount of memory needed to handle the HTTP requests and responses under heavy load can be huge, and it made more sense to increase the shared memory pool size than the private ones. The drawback is the cost of the SHM locking for each operation. Another option could be to allocate a new dedicated memory pool to each HTTP worker (or a shared one amongst the workers), this is now easily doable with the new kamailio memory API.
So, in a nutshell, to use both modules at the same time, libcurl initialization should be done in a shared API that would ensure that the initialization is done only once per process. To do this, we also have to agree on the memory API used by libcurl. In your opinion(s), what is the best alternative: libc's malloc, SHM, PKG, dedicated (private / shared) pool?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-173998873
Isn't curl using the system memory by default? It should not affect the pkg usage.
On the other hand, I don't think people will use the two modules discussed here at the same time. if there is a conflict in initializing the curl library, then utils and xcap_client are also using it.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-174000107
It is using system memory by default indeed, but we preferred kamailio's pools, to have a better view/control on what is going on.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-174001794
OK, maybe would be good to have a mod param option to control what mem pool (system or kamailio) is going to be use.
I think the module should be merged, then renamed to http_client or httpc.
If combining with curl module proves necessary over the time, then the curl module can end up being just a wrapper to libcurl, with the other modules (httpc, xcap_client) binding internally to curl module.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-174005844
Some commits following received suggestions: - allow global and per query cert and key configuration - memory manager configuration - allow to send a query without suspending the transaction
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-176125305
Trying to summarize the discussion from yesterday in Brussels at Fosdem between @oej, @grumvalski, @camilleoudot & all:
* https://twitter.com/giavac/status/693809404177715202
The main points (if I forgot something or misunderstood, just add more):
* rename the modules (something starting with http...) * rename the functions exported by modules not to have conflicts when using both modules at the same time * aim at a merge in the future, probably not before 4.4
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#issuecomment-177882747
Merged #480.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/480#event-537615394
-
Camille Oudot -
Daniel-Constantin Mierla
-
Federico Cabiddu
-
Federico Cabiddu -
Hugh Waite
-
Olle E. Johansson