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

[David Kupka]

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.

-- 
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0129.1-schema-cache-Fallback-to-en_us-when-locale-is-not-av.patch
Type: text/x-patch
Size: 1328 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160818/587535b9/attachment.bin>