TELECOM-11880: rest client refactor #108
Conversation
davidtrihy-genesys
requested review from
benceszigeti,
bcnewlin,
SubhashC37,
evan-maloney-genesys,
DHennessyGenesys and
sekharp-genesys
davidtrihy-genesys
changed the title
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
from
752f86f to
bd4e518
Compare
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
from
bd4e518 to
e61655f
Compare
| @@ -104,6 +114,16 @@ static int w_async_rest_put(struct sip_msg *msg, async_ctx *ctx, | |||
| str *url, str *body, str *_ctype, pv_spec_t *body_pv, | |||
| pv_spec_t *ctype_pv, pv_spec_t *code_pv); | |||
|
|
|||
| // Temporary to expose in script | |||
There was a problem hiding this comment.
Just for the script to fallback to when the feature toggle is on, I haven't changed the sync transfer but it needs to be here for the script before we can switch to the modparam
| @@ -139,6 +159,25 @@ static const acmd_export_t acmds[] = { | |||
| {CMD_PARAM_VAR,0,0}, | |||
| {CMD_PARAM_VAR|CMD_PARAM_OPT,0,0}, | |||
| {CMD_PARAM_VAR|CMD_PARAM_OPT,0,0}, {0,0,0}}}, | |||
| {"rest_get_v2",(acmd_function)w_async_rest_get_v2, { | |||
There was a problem hiding this comment.
Same as above, I've exposed the v2 functions as their own functions so we can feature toggle them in the script and eventually just used the modparam
| @@ -169,6 +208,28 @@ static const cmd_export_t cmds[] = { | |||
| {CMD_PARAM_VAR|CMD_PARAM_OPT,0,0}, | |||
| {CMD_PARAM_VAR|CMD_PARAM_OPT,0,0}, {0,0,0}}, | |||
| ALL_ROUTES}, | |||
| {"rest_get_v2",(cmd_function)w_rest_get, { | |||
There was a problem hiding this comment.
This is the script functions for the sync transfer for the script, will be removed eventually
| @@ -190,6 +251,72 @@ static const trans_export_t trans[] = { | |||
| {{0,0},0,0} | |||
| }; | |||
|
|
|||
| static int warm_pool_urls(modparam_t type, void *val) { | |||
There was a problem hiding this comment.
Given the modparam modparam("rest_client", "warm_pool_urls", "http://127.0.0.1:1080/normal,1000") this builds a simple linked list of URL and integer, this is done in the main process during startup and the list is available in all forked processes, even though it uses package memory and not shared memory it can still deallocate the block in the forked processes and we don't have to mess around with locking the shared memory
| @@ -298,6 +429,16 @@ static int cfg_validate(void) | |||
| return 1; | |||
| } | |||
|
|
|||
| static int get_fd_limit(void) { | |||
| if (getrlimit(RLIMIT_NOFILE, &lim) < 0) { | |||
There was a problem hiding this comment.
This allows to get file descriptor ulimit from the OS
modules/rest_client/rest_methods.c
Outdated
There was a problem hiding this comment.
_v2 functions are the new ones, to keep things clean and allow us to migrate to the new REST easier I have duplicated the functions and put in the changes in the duped ones, will homogenize in future PR
The code here is built to take advantage of modern OS multiplexing by allowing sockets to be created past the 1024 FD limit of the Posix standard.
The changes here include a modparam
modparam("rest_client", "warm_pool_urls", "http://127.0.0.1:1080/normal,1000")which will open 1000 sockets on startup of any process that uses the rest_client and keep them open, requires the server connecting to send keep alivesThis drastically improves response time of the apllication, I can run 2000 concurrent OPTIONS requests on a single process and get correct responses from the route after this change
The change now overwrites the existing client changes but the idea moving forward is to expose another async rest function that we can feature toggle so we can switch between them.
There are some curl options that need to be exposed as modparams which can be done after discussion of this change