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

[Martin Basti]

On 18.11.2015 17:58, Rob Crittenden wrote:
> 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
>
Yes, thank you Petr to catch this.
https://fedorahosted.org/freeipa/ticket/5463