TELECOM-11880: rest client refactor #108
Conversation
davidtrihy-genesys
requested review from
DHennessyGenesys,
SubhashC37,
bcnewlin,
benceszigeti,
evan-maloney-genesys and
sekharp-genesys
davidtrihy-genesys
changed the title
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
2 times, most recently
from
bd4e518 to
e61655f
Compare
| 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
| {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
| {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
| {{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
| } | ||
|
|
||
| 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
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
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
from
c72c51f to
931f4f9
Compare
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
from
931f4f9 to
25ccc98
Compare
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
3 times, most recently
from
54d1c44 to
df1eaa0
Compare
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
3 times, most recently
from
5664352 to
409ff33
Compare
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
from
409ff33 to
f544d5b
Compare
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
from
f544d5b to
4494dd7
Compare
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
3 times, most recently
from
6b1be77 to
d214fab
Compare
davidtrihy-genesys
force-pushed
the
TELECOM-11880
branch
from
d214fab to
f98c94a
Compare
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