[kamailio/kamailio] After commit ce8d36f latency_limit_db requires us instead of ms (#1956)
17 May
2019
17 May
'19
6:37 p.m.
Long time ago I have added in the configuration:
latency_limit_db=500
After the upgrade from version 5.1.7 to version 5.1.8, kamailio started printing a lot of
log lines related to db slow query:
<core> [db_query.c:68]: db_do_submit_query(): alert - query execution too long [1136
us]
<core> [db_query.c:68]: db_do_submit_query(): alert - query execution too long [914
us]
<core> [db_query.c:68]: db_do_submit_query(): alert - query execution too long [741
us]
<core> [db_query.c:68]: db_do_submit_query(): alert - query execution too long [621
us]
It seems that after the upgrade the latency_limit_db parameter configuration changed from
milliseconds to microseconds.
Here the commit that probably caused the issue:
https://github.com/kamailio/kamailio/commit/ce8d36f301d7688255597a67693e47d…
If a fix is released, please backport it till 5.1 branch.
Thank you.
Marco
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956
17 May
17 May
8:15 p.m.
Hello,
the diff looks like a deliberate change from ms to us.Lets wait for the author of the
change to comment.
Cheers,
Henning
Am 17.05.19 um 17:37 schrieb Marco Capetta:
Long time ago I have added in the configuration:
latency_limit_db=500
After the upgrade from version 5.1.7 to version 5.1.8, kamailio started printing a lot of
log lines related to db slow query:
[db_query.c:68]: db_do_submit_query(): alert - query execution too long [1136 us]
[db_query.c:68]: db_do_submit_query(): alert - query execution too long [914 us]
[db_query.c:68]: db_do_submit_query(): alert - query execution too long [741 us]
[db_query.c:68]: db_do_submit_query(): alert - query execution too long [621 us]
It seems that after the upgrade the latency_limit_db parameter configuration changed from
milliseconds to microseconds.
Here the commit that probably caused the issue:
ce8d36f<https://github.com/kamailio/kamailio/commit/ce8d36f301d768825559…
If a fix is released, please backport it till 5.1 branch.
Thank you.
Marco
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on
GitHub<https://github.com/kamailio/kamailio/issues/1956?email_source=not…UN2FRQ>,
or mute the
thread<https://github.com/notifications/unsubscribe-auth/ABO7UZOPZP3CZCA…EQ>.
_______________________________________________
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org<mailto:sr-dev@lists.kamailio.org>
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
--
Henning Westerholt - https://skalatan.de/blog/
Kamailio services - https://skalatan.de/services
11:06 p.m.
That commit was done as a fix, because the previous solution didn't work in most of
the cases. It relied on the internal track of time, but in case of blocking, the signal to
update the value didn't happen, so my tests with latency for action (which used the
same mechanism as for db) was most of the time 0, even I explicitly used sleep(few
seconds). The unit ms -- not sure if previously was still expected to be micro-seconds or
mili-seconds. The origin was somehow related to benchmark module which has micro-seconds
accuracy (or nano-seconds if compiled with librt), but to make the measurements
internally, not in config file, so I had in mind micro-seconds. If people consider it to
be something breaking what they expected or they were used to (but again, it was not
actually working), we can think of some option to add for time unit.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956#issuecomment-493582670
11:24 p.m.
The unit 'ms' is usually milli-seconds. Adding a config option for a time unit is
IMHO a bit overkill, the easiest way would be to add a multiplication by 1000. Technically
it is a regression in a stable release, I think. I can also take care of that tomorrow, no
worry.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956#issuecomment-493587991
11:32 p.m.
It is no regression as it was not working and it was expected to print what benchmark does
when used in config file.
This features was added not to enclose each action in kamailio confg between a
bm_start_time() and bm_log_time(), extended for db queries for better way to identify the
slow operations.
So the actually expectation was to print micro-seconds as benchmakr does. Therefore
multiplying with 1000 is not a proper fix here, it must stay microseconds in its base,
with option to change if people want (or they expected) a different unit. That can be made
default to miliseconds for old editions and let to microseconds for master branch. But I
would go and ask in the community and see what most of them expected there as time unit.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956#issuecomment-493590496
18 May
18 May
12:27 a.m.
No need to discuss a lot about it. :-) I just looked into the documentation, both
latency_limit_db and latency_limit_action are documented with milliseconds
[link](https://www.kamailio.org/wiki/cookbooks/5.2.x/core#latency_limit_db). But if
microseconds were actually defined in the code, then it is just a spelling error in the
docs.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956#issuecomment-493604555
29 May
29 May
11:06 p.m.
@miconda - did we reached any conclusion here? Should the code be fixed or the
documentation?
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956#issuecomment-497089361
30 May
30 May
10:38 a.m.
@henningw - it was left for the others to decide if they want a param in the stable
branches to make it milliseconds, otherwise it should stay microseconds, like benchmark
module does.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956#issuecomment-497237004
1 Jul
1 Jul
3:58 p.m.
Just tested the upgrade to kamailio 5.2.3 and in fact now both the parameters
(latency_limit_db and latency_limit_action) expect a micro-seconds value.
I suggest to leave the code as is and instead fix the documentation, currently it
suggesting to use milli-seconds.
Thanks
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956#issuecomment-507253817
2 Jul
2 Jul
10:17 p.m.
Closed #1956.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956#event-2455404247
10:17 p.m.
fixed in wiki for devel, 5.2 and 5.1.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1956#issuecomment-507810087
2000
days inactive
2046
days old
10 comments
4 participants
participants (4)
-
Daniel-Constantin Mierla -
Henning Westerholt -
Henning Westerholt
-
Marco Capetta