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

[Jan Cholasta]

On 17.8.2016 13:21, David Kupka wrote:
> On 08/08/16 13:26, Jan Cholasta wrote:
>> 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
>>
> Updated patches attached.

Thanks, ACK.

Pushed to master: 6e6cbda036559e741ead0ab5ba18b0be0b41621e

-- 
Jan Cholasta