async_http: non-blocking HTTP client module

[grumvalski]

This new module, based on libevent and cURL multi interface, implements non blocking HTTP queries.

[oej]

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.

[oej]

Having said that I agree that we need async HTTP :-)

[kamailio-sync]

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
#480 (comment).

---

sr-dev mailing list
sr-dev@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev

[miconda]

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, ...

[que273]

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

[grumvalski]

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
#480 (comment).

[grumvalski]

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
  • e.g. for sending logging/notifications where result is not
    important
  • 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.

[oej]

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!

[camilleoudot]

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?

[miconda]

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.

[camilleoudot]

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.

[miconda]

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.

[grumvalski]

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

[miconda]

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