2 May
2012
2 May
'12
3:33 p.m.
Hello,
looking at ndb_redis module source code I see replies are not freed when
module is destroyed.
Also a reply is only freed when another one with same name is performed.
This could cause problems if an user would ask for
a lot of different named replies.
IMHO freeing reply list when module is destroyed and creating a new
redisc_free_reply function would solve this.
I attach a patch that compiles but not tested to show the idea.
If you agree I can perform the task, I suppose redisc_free_reply
function should also be exported in cmd_export_t module structure.
This solution would be compatible with old ndb redis module configuration.
Regards,
Vicente.
2 May
2 May
3:38 p.m.
Ops, sorry, I send a wrong patch, that one creates an API for ndb_redis
module.
The patch attached now is the correct one.
Regards,
Vicente.
On 05/02/2012 09:33 PM, Vicente Hernando wrote:
Hello,
looking at ndb_redis module source code I see replies are not freed
when module is destroyed.
Also a reply is only freed when another one with same name is
performed. This could cause problems if an user would ask for
a lot of different named replies.
IMHO freeing reply list when module is destroyed and creating a new
redisc_free_reply function would solve this.
I attach a patch that compiles but not tested to show the idea.
If you agree I can perform the task, I suppose redisc_free_reply
function should also be exported in cmd_export_t module structure.
This solution would be compatible with old ndb redis module
configuration.
Regards,
Vicente.
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
3 May
3 May
7:29 a.m.
Hello,
thanks for the patch -- when the module is destroyed, practically is the shut down of
kamailio, when the entire memory used is freed (pkg and shm) as well as the one from the
system by the system manager. Anyhow, for a clean shut down, is good to have it.
One thing is not clear, where the function redisc_free_reply(str *name) is used? The patch
seems just to define it.
Cheers,
Daniel
On 5/2/12 9:38 PM, Vicente Hernando wrote:
Ops, sorry, I send a wrong patch, that one creates an
API for
ndb_redis module.
The patch attached now is the correct one.
Regards,
Vicente.
On 05/02/2012 09:33 PM, Vicente Hernando wrote:
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Hello,
looking at ndb_redis module source code I see replies are not freed
when module is destroyed.
Also a reply is only freed when another one with same name is
performed. This could cause problems if an user would ask for
a lot of different named replies.
IMHO freeing reply list when module is destroyed and creating a new
redisc_free_reply function would solve this.
I attach a patch that compiles but not tested to show the idea.
If you agree I can perform the task, I suppose redisc_free_reply
function should also be exported in cmd_export_t module structure.
This solution would be compatible with old ndb redis module
configuration.
Regards,
Vicente.
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
8 a.m.
Hello,
On 05/03/2012 01:29 PM, Daniel-Constantin Mierla wrote:
Hello,
thanks for the patch -- when the module is destroyed, practically is the shut down of
kamailio, when the entire memory used is freed (pkg and shm) as well as the one from the
system by the system manager. Anyhow, for a clean shut down, is good to have it.
One thing is not clear, where the function redisc_free_reply(str *name) is used? The
patch seems just to define it.
The patch is not finished, only to show a possible
solution idea.
Right now, if a user performs:
redis_cmd("srvN", "GET foo", "r");
a redisc_reply_t structure is created and assigned to name "r". If user
performs again:
redis_cmd("srvN", "GET bar", "r");
then old redisk_reply_t.rplRedis is freed with freeReplyObject and
hiredis creates a new redisReply for rplRedis field.
No problem here, then when kamailio closes that last structure would be
freed.
However, if user would perform:
redis_cmd("srvN", "GET foo", "r1");
redis_cmd("srvN", "GET foo", "r2");
redis_cmd("srvN", "GET foo", "r3");
...
those structures would not be freed from memory.
In my opinion if a user creates a lot of different replies, it should be
his responsibility to free them using e.g:
redis_free("r1")
redis_free("r2")
Most users will not need this. I am planning exporting ndb_redis
functions in an API, and use them heavily in another new module.
Regards,
Vicente.
Cheers,
Daniel
On 5/2/12 9:38 PM, Vicente Hernando wrote:
Ops, sorry, I send a wrong patch, that one
creates an API for
ndb_redis module.
The patch attached now is the correct one.
Regards,
Vicente.
On 05/02/2012 09:33 PM, Vicente Hernando wrote:
--
Daniel-Constantin Mierla -http://www.asipto.com
http://twitter.com/#!/miconda -http://www.linkedin.com/in/miconda Hello,
looking at ndb_redis module source code I see replies are not freed
when module is destroyed.
Also a reply is only freed when another one with same name is
performed. This could cause problems if an user would ask for
a lot of different named replies.
IMHO freeing reply list when module is destroyed and creating a new
redisc_free_reply function would solve this.
I attach a patch that compiles but not tested to show the idea.
If you agree I can perform the task, I suppose redisc_free_reply
function should also be exported in cmd_export_t module structure.
This solution would be compatible with old ndb redis module
configuration.
Regards,
Vicente.
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
10 May
10 May
4:58 a.m.
Hello,
On 05/03/2012 02:00 PM, Vicente Hernando wrote:
Hello,
On 05/03/2012 01:29 PM, Daniel-Constantin Mierla wrote:
> Hello,
>
> thanks for the patch -- when the module is destroyed, practically is the shut down of
kamailio, when the entire memory used is freed (pkg and shm) as well as the one from the
system by the system manager. Anyhow, for a clean shut down, is good to have it.
>
> One thing is not clear, where the function redisc_free_reply(str *name) is used? The
patch seems just to define it.
Following your point I stripped the part outside
redis_destroy function.
This patch frees _redisc_rpl_list at redis_destroy function.
I think I will submit the other part to tracker as an improvement when
finished, so not to be forgotten after the freeze.
Regards,
Vicente.
The patch is not finished, only to show a possible
solution idea.
Right now, if a user performs:
redis_cmd("srvN", "GET foo", "r");
a redisc_reply_t structure is created and assigned to name "r". If
user performs again:
redis_cmd("srvN", "GET bar", "r");
then old redisk_reply_t.rplRedis is freed with freeReplyObject and
hiredis creates a new redisReply for rplRedis field.
No problem here, then when kamailio closes that last structure would
be freed.
However, if user would perform:
redis_cmd("srvN", "GET foo", "r1");
redis_cmd("srvN", "GET foo", "r2");
redis_cmd("srvN", "GET foo", "r3");
...
those structures would not be freed from memory.
In my opinion if a user creates a lot of different replies, it should
be his responsibility to free them using e.g:
redis_free("r1")
redis_free("r2")
Most users will not need this. I am planning exporting ndb_redis
functions in an API, and use them heavily in another new module.
Regards,
Vicente.
Cheers,
Daniel
On 5/2/12 9:38 PM, Vicente Hernando wrote:
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev Ops, sorry, I send a wrong patch, that one
creates an API for
ndb_redis module.
The patch attached now is the correct one.
Regards,
Vicente.
On 05/02/2012 09:33 PM, Vicente Hernando wrote:
--
Daniel-Constantin Mierla -http://www.asipto.com
http://twitter.com/#!/miconda -http://www.linkedin.com/in/miconda Hello,
looking at ndb_redis module source code I see replies are not freed
when module is destroyed.
Also a reply is only freed when another one with same name is
performed. This could cause problems if an user would ask for
a lot of different named replies.
IMHO freeing reply list when module is destroyed and creating a new
redisc_free_reply function would solve this.
I attach a patch that compiles but not tested to show the idea.
If you agree I can perform the task, I suppose redisc_free_reply
function should also be exported in cmd_export_t module structure.
This solution would be compatible with old ndb redis module
configuration.
Regards,
Vicente.
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
4480
days inactive
4488
days old
4 comments
2 participants
participants (2)
-
Daniel-Constantin Mierla -
Vicente Hernando