[Freeipa-devel] [PATCH 0112-7] Speeding up cli help

[Jan Cholasta]

On 4.8.2016 16:32, David Kupka wrote:
> On 03/08/16 16:33, Jan Cholasta wrote:
>> On 3.8.2016 16:23, David Kupka wrote:
>>> On 21/07/16 10:12, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> On 20.7.2016 14:32, David Kupka wrote:
>>>>> On 15/07/16 12:53, David Kupka wrote:
>>>>>> Hello!
>>>>>>
>>>>>> After Honza introduced thin client that builds plugins and commands
>>>>>> dynamically from schema client became much slower. This is only
>>>>>> logical,
>>>>>> instead of importing a module client now must fetch the schema from
>>>>>> server, parse it and instantiate the commands using the data.
>>>>>>
>>>>>> First step to speed it up was addition of schema cache to client.
>>>>>> That
>>>>>> removed the RTT and download time of fetching schema every time.
>>>>>>
>>>>>> Now the most time consuming task became displaying help for lists of
>>>>>> topics and command and displaying individual topics. This is simply
>>>>>> because of the need to instantiate all the commands to find the
>>>>>> relations between topics and commands.
>>>>>>
>>>>>> All the necessary bits for server commands and topics are already in
>>>>>> the
>>>>>> schema cache so we can skip this part and generate help from it,
>>>>>> right?
>>>>>> Not so fast!
>>>>>>
>>>>>> There are client plugins with commands and topics. So we can generate
>>>>>> basic bits (list of all topics, list of all commands, list of
>>>>>> commands
>>>>>> for each topic) from schema and store it in cache. Then we need to go
>>>>>> through all client plugins and get similar bits for client plugins.
>>>>>> Then
>>>>>> we can merge and print.
>>>>>>
>>>>>> Still the client response is not as fast as before and I this it even
>>>>>> can't be. Also first time you display particular topic or list takes
>>>>>> longer because it must be freshly generated and stored in cache for
>>>>>> next
>>>>>> use. And this is what the attached patches do.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/6048
>>>>>
>>>>> Reimplemented so there is no need to distinguish client plugins and
>>>>> remote plugins.
>>>>> The main idea of this approach is to avoid creating instances of the
>>>>> commands just to get the information about topic, name and summary
>>>>> needed for displaying help. Instead class properties are used to
>>>>> access
>>>>> the information directly in schema.
>>>>
>>>> Patch 0112:
>>>>
>>>> I think this would better be done in Schema.read_namespace_member,
>>>> because Schema is where all the state is.
>>>>
>>>> (BTW does _SchemaNameSpace.__getitem__ raise KeyError for non-existent
>>>> keys? It looks like it doesn't.)
>>>>
>>>>
>>>> Patch 0113:
>>>>
>>>> How about setting _schema_cached to False in Schema.__init__() rather
>>>> that getattr()-ing it in _ensure_cached()?
>>>>
>>>>
>>>> Patch 0116:
>>>>
>>>> ClientCommand.doc should be a class property as well, otherwise
>>>> .summary
>>>> won't work on it correctly.
>>>>
>>>> _SchemaCommand.doc should not be a property, as it's not needed for
>>>> .summary to work on it correctly.
>>>>
>>>>
>>>> Otherwise works fine for me.
>>>>
>>>> Honza
>>>>
>>>
>>> Updated patches attached.
>>
>> Thanks, ACK.
>>
>> Pushed to master: 229e2a1ed9ea9877cb5e879fadd99f9040f77c96
>>
>
> I've made and reviewer overlooked some errors. Attached patches fix them.

Shame on me.

Anyway,

1) When schema() raises SchemaUpToDate, the current code explodes:

ipa: ERROR: KeyError: 'fingerprint'
Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1348, in run
     api.finalize()
   File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 707, 
in finalize
     self.__do_if_not_done('load_plugins')
   File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 422, 
in __do_if_not_done
     getattr(self, name)()
   File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 585, 
in load_plugins
     for package in self.packages:
   File "/usr/lib/python2.7/site-packages/ipalib/__init__.py", line 919, 
in packages
     ipaclient.remote_plugins.get_package(self),
   File 
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/__init__.py", 
line 92, in get_package
     plugins = schema.get_package(api, server_info, client)
   File 
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py", 
line 558, in get_package
     fingerprint = str(schema['fingerprint'])
   File 
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py", 
line 483, in __getitem__
     return self._dict[key]
KeyError: 'fingerprint'


2) We read server info every time get_package() is called. It should be 
cache similarly to how the schema is cached in the api object.


3) Some of the commands are still fully initialized during "ipa help 
commands".


Honza

-- 
Jan Cholasta