[Freeipa-devel] Caching ldap limits for whole connection (performance)

[Rob Crittenden]

Martin Basti wrote:
> 
> 
> On 18.11.2015 17:34, Martin Basti wrote:
>>
>>
>> On 18.11.2015 14:25, Petr Vobornik wrote:
>>> On 11/17/2015 10:37 AM, Martin Basti wrote:
>>>>
>>>>
>>>> On 16.11.2015 20:18, Rob Crittenden wrote:
>>>>> Martin Basti wrote:
>>>>>>
>>>>>>
>>>>>> On 16.11.2015 18:57, Martin Basti wrote:
>>>>>>> How does this code work (IMO it doesn't), ldap2.py
>>>>>>>
>>>>>>>     def find_entries(self, filter=None, attrs_list=None,
>>>>>>> base_dn=None,
>>>>>>>                      scope=_ldap.SCOPE_SUBTREE, time_limit=None,
>>>>>>>                      size_limit=None, search_refs=False,
>>>>>>> paged_search=False):
>>>>>>>
>>>>>>>         def _get_limits():
>>>>>>>             """Get configured global limits, caching them for more
>>>>>>> calls"""
>>>>>>>             if not _lims:
>>>>>>>                 config = self.get_ipa_config()
>>>>>>>                 _lims['time'] = int(config.get('ipasearchtimelimit',
>>>>>>> [None])[0])
>>>>>>>                 _lims['size'] =
>>>>>>> int(config.get('ipasearchrecordslimit', [None])[0])
>>>>>>>             return _lims
>>>>>>>         _lims = {}
>>>>>>>
>>>>>>>         if time_limit is None:
>>>>>>>             time_limit = _get_limits()['time']
>>>>>>>         if size_limit is None:
>>>>>>>             size_limit = _get_limits()['size']
>>>>>>>
>>>>>>> Code above is supposed to do caching, but it doesn't do it. This
>>>>>>> might
>>>>>>> work if _lims were self._lims.
>>>>>>> I tried similar code to test this behavior:
>>>>>>>
>>>>>>> class test:
>>>>>>>     def __init__(self):
>>>>>>>        pass
>>>>>>>
>>>>>>>     def cached_call(self):
>>>>>>>        """configured global limits"""
>>>>>>>        _lims = {}
>>>>>>>        def _get_limits():
>>>>>>>            if not _lims:
>>>>>>>                _lims['t']='oujeee'
>>>>>>>                print 'getting limits'
>>>>>>>            return _lims
>>>>>>>
>>>>>>>        print "Limits:", _get_limits()['t']
>>>>>>>
>>>>>>> t = test()
>>>>>>> t.cached_call()
>>>>>>> t.cached_call()
>>>>>>> t.cached_call()
>>>>>>> t.cached_call()
>>>>>>>
>>>>>>> Output:
>>>>>>> $ python testcaching.py
>>>>>>> Limits: getting limits
>>>>>>> oujeee
>>>>>>> Limits: getting limits
>>>>>>> oujeee
>>>>>>> Limits: getting limits
>>>>>>> oujeee
>>>>>>> Limits: getting limits
>>>>>>> oujeee
>>>>>>>
>>>>>>> So it does not do caching, or am I wrong?
>>>>>>> Martin^2
>>>>>>>
>>>>>> That code works, the whole caching is just for the second call of
>>>>>> _get_limits()
>>>>>>
>>>>>> Can we instead just caching limits for second use, do caching for
>>>>>> whole
>>>>>> connection?
>>>>>> This may be effective for methods/commands that calls search and show
>>>>>> several times.
>>>>>>
>>>>>> Is there something that prevents us to do that?
>>>>>>
>>>>>
>>>>> It already is cached. See get_ipa_config().
>>>>>
>>>>> rob
>>>> I missed that part there, thank you.
>>>> Martin
>>>>
>>>
>>> I tried user_add and according to access log(
>>> http://fpaste.org/291835/44785307/ ) it alone does 13 searches for
>>> ipa config:
>>>
>>> SRCH base="cn=ipaconfig,cn=etc,dc=example.com" scope=0
>>> filter="(objectClass=*)" attrs=ALL
>>>
>>> So I think it is not working correctly.
>>
>> I did testing, I put following debug prints into get_ipa_config method:
>>
>> +       print("call: get_ipa_config")
>>         try:
>>             config_entry = getattr(context, 'config_entry')
>>             if config_entry.conn is self.conn:
>>                 return config_entry
>> +            print("get_ipa_config: different connection!")
>>         except AttributeError:
>>             # Not in our context yet
>>             pass
>>
>> then I restarted apache, executed "ipa user-add test2" and result is
>> here: http://fpaste.org/291926/64204144/
>>
>> what means that the check in get_ipa_config is wrong, or ipa framework
>> always creates a new connection.
>> Except the first call that does not have cached value in context,
>> every other call asked directly LDAP for the value.
>>
> The comparation of connections in get_ipa_config does not work.
> http://fpaste.org/291937/64863144/
> 
> I added repr(connection) of cached and current connection, and self.conn
> is still the same but *is* operator returned failed.
> 

Nice catch. I guess the next step is to see when this was broken. I had
a 4.1.4 system lying around and that works so this is a fairly recent issue.

I'd suggest expanding your debugging to show the two connections, maybe
that will provide some clues.

IIRC there are tests to ensure that context.conn is being set but
perhaps something changed.

rob