[Kamailio-Devel] SF.net SVN: openser:[5764] trunk

Daniel-Constantin Mierla miconda at gmail.com
Thu Mar 26 15:25:03 CET 2009


On 03/26/2009 04:14 PM, Ovidiu Sas wrote:
> 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.
>   
Just several statistics are exported by core (those prefixed with core::).

You can move it better to statistics.{c,h} if these are new statistics 
exported by core, but I have not seen changes to core_stats.c -- where 
the table for core exported statistics is.

>   
>>> 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.
>   

Better let's find the proper place for this code and move it directly 
there, rather than simply revert. For example I would need access to 
cpuload details, code which is now in ratelimit module. Maybe we can 
combine the two in a single place accessible by the two modules.

Thanks,
Daniel

>
> 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/
>>
>>
>>     

-- 
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