[Kamailio-Devel] SF.net SVN: openser:[5764] trunk
Ovidiu Sas
osas at voipembedded.com
Thu Mar 26 15:14:57 CET 2009
On Thu, Mar 26, 2009 at 9:40 AM, Daniel-Constantin Mierla
<miconda at gmail.com> wrote:
> Hello Ovidiu,
>
> On 03/26/2009 03:23 PM, Ovidiu Sas wrote:
>>
>> Hello Daniel,
>>
>> The code dealing with statistic is pretty much zero conf, that's why I
>> think it is more appropriate to have it in a library rather then a
>> module.
>> Yes, there are many modules relying on tm, but tm is not a simple
>> piece of code and it has lot's of tuning parameters.
>>
>> Keeping the statistics code in a library makes life easier for both
>> users and devs:
>> - users don't need to pre-load a separate module;
>> - devs don't need to load an external API and check if the module is
>> loaded or not.
>>
>> To summarize, if code is required by several modules and no
>> configuration is need it, I think it will be more appropriate to keep
>> it in a library rather then a module.
>
> there is no library support in K. It is why presence implementation has some
> generic modules like presence and pua. They export functions for the rest of
> presence modules. This code is hardcoding /proc/net path.
>
> What I said is that in K such case is solved by modules exported API. We do
> not push code from modules to core for corner specific cases.
All the k statistics are still in core, that's why I felt that I can
push this code that belongs to statistics into the core.
>> And on the other side, if the
>> code require configuration, then it should be a module.
>>
>> Of course, this is just my view and if the community feels that we
>> should merge the core statistics into the statistics module, then we
>> will do it :)
>>
>
> I believe this is a wrong approach and thinking. Pushing code somewhere in
> core without proper discussion and then invoking to wait and see how
> community feels about will end up in big coherence problems. We agreed that
> additions to core have to be analyzed and discussed on devel lists.
Sorry about that. I thought that adding some stats code to the
existing core stats code will not break the core rule. I will revert
the two commits soon.
Regards,
Ovidiu Sas
> Thanks,
> Daniel
>>
>> Regards,
>> Ovidiu Sas
>>
>> On Thu, Mar 26, 2009 at 5:46 AM, Daniel-Constantin Mierla
>> <miconda at gmail.com> wrote:
>>
>>>
>>> Hello Ovidiu,
>>>
>>> On 03/26/2009 11:36 AM, Ovidiu Sas wrote:
>>>
>>>>
>>>> Hello Daniel,
>>>>
>>>> On Thu, Mar 26, 2009 at 4:57 AM, Daniel-Constantin Mierla
>>>> <miconda at gmail.com> wrote:
>>>>
>>>>
>>>>>
>>>>> Hello Ovidiu,
>>>>>
>>>>> On 03/26/2009 12:35 AM, Ovidiu Sas wrote:
>>>>>
>>>>>
>>>>>>
>>>>>> Hello Jan,
>>>>>>
>>>>>> Those functions are exporting statistics related to the socket (the
>>>>>> number of bytes in the socket queue).
>>>>>> The ratelimit module has a new algorithm which relies on the number of
>>>>>> bytes in a socket queue.
>>>>>> If the number of bytes waiting in the queue is reaching the limit, the
>>>>>> server can reject incoming requests.
>>>>>> More info here:
>>>>>> http://openser.svn.sourceforge.net/openser/?rev=5765&view=rev
>>>>>>
>>>>>> The code was simply moved from the snmpstats module into the socket
>>>>>> files.
>>>>>> Statistics belonging to the sockets should not belong to a particular
>>>>>> module, as other modules may need to use it (in my case: ratelimit).
>>>>>> That was the reason for moving the particular piece of code into the
>>>>>> core socket file.
>>>>>>
>>>>>> Is there a specific place to compute statistics in sr core?. If yes,
>>>>>> we should move the code there and expose it to all the modules that
>>>>>> are requesting it.
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> I think this code belongs better to the statistics module for now in
>>>>> Kamailio and maybe moved later to a library in sr. The general idea,
>>>>> even
>>>>> with Kamailio, was to move the specific code out of the core. There is
>>>>> lot
>>>>> of undergoing work to integrate K with sip-router core, let's discuss
>>>>> any
>>>>> addition to the core in a while. There are very few left modules to be
>>>>> integrated, therefore should not take that long.
>>>>>
>>>>> I am thinking in creating a new module where to add cfg extensions in
>>>>> Kamailio core which are not used often. Not sure what name to give
>>>>> here...
>>>>>
>>>>>
>>>>
>>>> For now, we can move the code into kcore library and later on into a
>>>> specific library.
>>>> I don't think it's good idea to move the code into the statistics
>>>> module. The statistics module is just a framework for user defined
>>>> statistics.
>>>>
>>>
>>> It is now, this does not mean will stay so forever. However, these are
>>> statistics. If a module needs access to tm function, that function is
>>> exported via api not moved in core. There are dependencies between
>>> modules
>>> to avoid making core fat, thing that happened before.
>>>
>>>
>>>
>>>>
>>>> Moving the code there, will create additional
>>>> dependencies for snmpstats and ratelimit modules. The reason for
>>>> moving the code out of the snmpstats module was to avoid new module
>>>> inter-dependencies.
>>>>
>>>>
>>>>
>>>
>>> You add code in core instead of loading a module for cases that might not
>>> be
>>> so common. How many would need the new algorithm in ratelimit and how
>>> many
>>> use the snmpstats module. You can update ratelimit to disable this
>>> algorithm
>>> if the module exporting the statistics (being it snmpstats or statistics)
>>> is
>>> not loaded. Same is with sl, if tm is loaded, send_reply() will check for
>>> transaction and use t_replay() if exists, otherwise will reply all the
>>> time
>>> stateless.
>>>
>>> Cheers,
>>> Daniel
>>>
>>>
>>>>>>
>>>>>> On Wed, Mar 25, 2009 at 6:18 PM, Jan Janak <jan at ryngle.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> What is the purpose of this change? I am asking because it breaks
>>>>>>> snmpstats in
>>>>>>> the kamailio-3.0 repository again.
>>>>>>>
>>>>>>> It seems to add two public functions, get_socket_list_from_proto and
>>>>>>> get_total_bytes_waiting. The 2nd function also seems to introduce
>>>>>>> extra
>>>>>>> static
>>>>>>> functions like parse_proc_net_line, get_used_waiting_queue, and
>>>>>>> match_ip_and_port.
>>>>>>>
>>>>>>> Do you believe that this belongs to the core, and especially in
>>>>>>> socket_info.c?
>>>>>>>
>>>>>>> Note also that parse_proc_net_line and get_used_waiting_queu rely on
>>>>>>> parsing
>>>>>>> files in /proc/net. These functions are not portable, most likely
>>>>>>> they
>>>>>>> will
>>>>>>> not work on non-linux based systems.
>>>>>>>
>>>>>>> Jan.
>>>>>>>
>>>>>>> On 24-03 22:54, Ovidiu Sas wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Revision: 5764
>>>>>>>> http://openser.svn.sourceforge.net/openser/?rev=5764&view=rev
>>>>>>>> Author: osas
>>>>>>>> Date: 2009-03-24 22:54:33 +0000 (Tue, 24 Mar 2009)
>>>>>>>>
>>>>>>>> Log Message:
>>>>>>>> -----------
>>>>>>>> Moving socket related code from snmpstats module into the core:
>>>>>>>> - other modules will be able to access socket related statistics
>>>>>>>> directly from core
>>>>>>>>
>>>>>>>> Modified Paths:
>>>>>>>> --------------
>>>>>>>> trunk/modules/snmpstats/alarm_checks.c
>>>>>>>> trunk/modules/snmpstats/openserObjects.c
>>>>>>>> trunk/modules/snmpstats/openserSIPPortTable.c
>>>>>>>> trunk/modules/snmpstats/snmpstats_globals.h
>>>>>>>> trunk/socket_info.c
>>>>>>>> trunk/socket_info.h
>>>>>>>>
>>>>>>>> Removed Paths:
>>>>>>>> -------------
>>>>>>>> trunk/modules/snmpstats/network_stats.c
>>>>>>>> trunk/modules/snmpstats/network_stats.h
>>>>>>>>
>>>>>>>>
>>>
>>> --
>>> Daniel-Constantin Mierla
>>> SIP Router Masterclass - Kamailio (OpenSER) Training
>>> http://www.asipto.com/index.php/sip-router-masterclass/
>>>
>>>
>>>
>>
>> _______________________________________________
>> Kamailio (OpenSER) - Devel mailing list
>> Devel at lists.kamailio.org
>> http://lists.kamailio.org/cgi-bin/mailman/listinfo/devel
>> http://lists.openser-project.org/cgi-bin/mailman/listinfo/devel
>>
>>
>
> --
> Daniel-Constantin Mierla
> SIP Router Masterclass - Kamailio (OpenSER) Training
> http://www.asipto.com/index.php/sip-router-masterclass/
>
>
More information about the Devel
mailing list