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

[Jan Cholasta]

On 18.8.2016 11:06, David Kupka wrote:
> On 17/08/16 14:17, Jan Cholasta wrote:
>> 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
>>
>
> When locale is not available setlocale() explodes. Handling this
> exception in the same way as in ipalib/rpc.py to behave consistently.

Works for me, ACK.

Pushed to master: b6d5ed139b261b5db078ab652d22ea1d3b8092d3

-- 
Jan Cholasta