Hi Daniel, thank you for looking into this. changes are now merged. Thanks ________________________________ From: Daniel-Constantin Mierla [[email protected]] Sent: Saturday, September 13, 2014 8:55 AM To: Luis Azedo; Kamailio (SER) - Development Mailing List; Waite, Hugh; Peter Dunkley Subject: Re: [sr-dev] kazoo module updates & documentation
Hello, ok, so I got back into the db_text code (as much as I wanted not to :-) ) and now it's clearer what's all about. There are practically two result structures, one intermediary built internally by db_text and from it, one built for the use in modules. I thought it is about a single structure that has now to be moved somewhere else. But with db_text, the first was in connection, the second was in db_res, so a new query would reset the intermediary one from connection. It makes sense to have both in one place, otherwise indeed can end up in issues with a new query before processing current result. You can go ahead and merge the patch. Regarding DB_CAP_FETCH is set in lib/srdb1 for the modules that export the fetch result function. Thanks, Daniel On 13/09/14 14:47, Luis Azedo wrote: Hi Daniel, since db_text doesn't support DB_CAP_FETCH i think this wouldn't be a problem. the internal structure result is saved in db1_res ptr field. I don't know about the other db modules, but a quick look at mysql db module seems to show a similar approach with db1_res ptr field, check db_mysql_free_result btw, db_fetch_next tests for DB_CAP_FETCH and i believe this is not set mysql db_init, am i missing something? ________________________________ From: Daniel-Constantin Mierla [[email protected]<mailto:[email protected]>] Sent: Saturday, September 13, 2014 1:17 AM To: Luis Azedo; Kamailio (SER) - Development Mailing List; Waite, Hugh; Peter Dunkley Subject: Re: [sr-dev] kazoo module updates & documentation Hello, this looks like a problem, indeed. Thanks for troubleshooting to get down to it. However, the solution to store the result in local variable is still not the good choice. That is because we have so called result fetch support, where the result from the query is not retrieved all at once from database connection, but you can specify the number of rows (e.g., you want to get in db_res only 1000 rows at the moment). Therefore, the result of the initial query will not be processed entirely if the second query is done over the same connection after retrieving only the first fetch_rows. I haven't looked deep at your patch to db_text, it might solve it for your module, but overall the problem remains with the other db modules (e.g., mysql, ...). We need to come up with a fix for everything affected. One solution is to update the presence module to use two connection, one not-pooled (so it will do always a new connection, not selecting from existing pool if db_url matches). IIRC, there is a similar case in another module, developed at that time by Peter or Hugh -- cc-ed them, hopefully they can share some insights quickly. I will look as well over the issue to see if there are alternatives to fix it. Cheers, Daniel On 13/09/14 03:59, Luis Azedo wrote: Hi Daniel, please take a look at msg_presentity_clean function in publish.c module presence. if the initial query returns a result if pres_notifier_processes = 0 (immediate notify) it calls publ_notify function in publ_notify function in notify.c if(p->event->agg_nbody) // dialoginfo aggregates the results { notify_body = get_p_notify_body(pres_uri, p->event , offline_etag, NULL); ..... get_p_notify_body queries the presentity table to get other records to aggregate body which will override the result in the connection. so the process is the same but it calls the query 2 times in a row without freeing the 1st result. ________________________________ From: [email protected]<mailto:[email protected]> [[email protected]<mailto:[email protected]>] on behalf of Daniel-Constantin Mierla [[email protected]<mailto:[email protected]>] Sent: Friday, September 12, 2014 12:15 PM To: Kamailio (SER) - Development Mailing List Subject: Re: [sr-dev] kazoo module updates & documentation Hello, changes to kazoo modules are ok, you can merge them. But the one for db_text I don't find it (yet) necessary, because it may solve an issue in a wrong way. I asked you to push the other patch on db_text related to blob type handling in freeing the value. That can be done. For the one storing the result on db_res variable, I will comment separately. Daniel On 12/09/14 20:55, Luis Azedo wrote: Hi, i would like to propose the small changes to the kazoo module in branch lazedo/kazoo to go into master. it's mostly code cleanup & dependency removal. also added the documentation on that branch and would appreciate any feedback. there is also a patch to the db_text module. the comment on that commit tells the reason for that patch. Thanks _______________________________________________ sr-dev mailing list [email protected]<mailto:[email protected]> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev -- Daniel-Constantin Mierla http://twitter.com/#!/miconda<http://twitter.com/#%21/miconda> - http://www.linkedin.com/in/miconda Next Kamailio Advanced Trainings 2014 - http://www.asipto.com Sep 22-25, Berlin, Germany -- Daniel-Constantin Mierla http://twitter.com/#!/miconda<http://twitter.com/#%21/miconda> - http://www.linkedin.com/in/miconda Next Kamailio Advanced Trainings 2014 - http://www.asipto.com Sep 22-25, Berlin, Germany -- Daniel-Constantin Mierla http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda Next Kamailio Advanced Trainings 2014 - http://www.asipto.com Sep 22-25, Berlin, Germany
_______________________________________________ sr-dev mailing list [email protected] http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
