# ssh performance - follow up **URL:** \ **Category:** Ansible Developer\ **Created:** [November 29, 2013, 12:46am UTC](https://forum.ansible.com/t/ssh-performance-follow-up/14028 "2013-11-29T00:46:13Z")\ **Posts on this page:** 4\ **Page:** 2
**Author:** ![Michael\_DeHaan2](https://avatars.discourse-cdn.com/v4/letter/m/da6949/32.png) [@Michael\_DeHaan2](https://forum.ansible.com/u/Michael_DeHaan2)\ **Post date:** [December 1, 2013, 7:01pm UTC](https://forum.ansible.com/t/ssh-performance-follow-up/14028/21 "2013-12-01T19:01:43Z")
Ok great, didn’t try yet 🙂 – Michael ---
**Author:** ![Jerome\_Wagner](https://avatars.discourse-cdn.com/v4/letter/j/76d3ee/32.png) [@Jerome\_Wagner](https://forum.ansible.com/u/Jerome_Wagner)\ **Post date:** [December 1, 2013, 8:47pm UTC](https://forum.ansible.com/t/ssh-performance-follow-up/14028/22 "2013-12-01T20:47:09Z")
Hello, looking for a solution on the logic for deciding what calls should be optimized or not, I have a few questions / suggestions : (warning: this is a bit long) A/ action & modules composability From what I understand, : (1) - an action\_plugin is allowed to invoke remote modules via runner.\_execute\_module (2) - an action\_plugin is allowed to call another action\_plugin via handler.run (only one case seen in script: which calls ‘raw’) (3) - the connection\_plugin accelerate calls \_execute\_module to launch the accelerate server - do you consider that case (2) and (3) are sufficiently degenerate as to be very rare ? - do you consider that case (2) is “legal” or should handler.run only be called by the runner and script: modified ? B/ creation and need for a temporary path The creation of a tmp path is decided in the runner just before handler.run (both normal & until case) It is not created if handler.NEEDS\_TMPPATH is set to False by the developer of the action plugin. a tmp path is also created by accelerate I did not look into this. except in the “accelerate” case, the tmp path is necessary : (a) - if an action\_plugin know that it will need to send tmp information to the remote host (b) - if a module needs to work in an ansible tmp dir (copy:?) (c) - when ansible wants to send the code of a module for execution currently, NEEDS\_PATH defaults to yes without distinction between (a) (b) and (c) the ssh\_alt branch show that (c) does not depend on the action\_plugin but on the capabilities of the connection\_plugin. so we could write : if getattr(handler, ‘NEEDS\_TMPPATH’, True) or not conn.has\_pipelining : tmp = self.\_make\_tmp\_path(conn) and expect that in the long run, NEEDS\_PATH express the real applicative need for a tmp path not taking into account the transfer of the module. then we could work on a module by module basis to set the real applicative value of NEEDS\_TMPPATH the only problem I have with this is that all operations on the tmp dir (chmod, removal…) should be guarded by the same test. This is a problem since runner.\_execute\_module does not have access to the original handler where NEEDS\_PATH is stored. The removal of the tmp path is also done in \_execute\_module so this is basically the same problem: how can we share this property of the action\_plugin up down to the execution of \_execute\_module. I could suggest we modify the prototype of action\_plugin.run to add (has\_remote\_tmp = False) so when calling handler.run, the runner could pass in this variable its final decision to create or not the tmp path. further calls to handler.run or \_execute\_module should forward this value along with the current persist\_files. if action modules have the right to call other action modules, we also need the “persist\_files” in ActionModule.run (cf raw call from script) so my suggestion : A] add a new function to ActionModule: def needs\_remote\_path(conn) where an ActionModule can express its applicative need for a tmp path. I transform NEEDS\_PATH into a function because depending on the capabilities of the connection, the ActionModule may or may not need a tmp path. This is for example true for the script: action. B] change the guard around \_make\_tmp\_path : has\_remote\_tmp = False if handler.needs\_remote\_path(conn) or not conn.has\_pipelining : tmp = self.\_make\_tmp\_path(conn) has\_remote\_tmp = True B] change prototypes of action\_plugin.run and runner.\_execute\_module ActionModule: def run(self, conn, tmp, module\_name, module\_args, inject, complex\_args=None, \*\*kwargs): becomes def run(self, conn, tmp, module\_name, module\_args, inject, complex\_args=None, has\_remote\_tmp=False, keep\_remote\_tmp=True, \*\*kwargs): runner : def \_execute\_module(self, conn, tmp, module\_name, args, async\_jid=None, async\_module=None, async\_limit=None, inject=None, persist\_files=False, complex\_args=None) becomes def \_execute\_module(self, conn, tmp, module\_name, args, async\_jid=None, async\_module=None, async\_limit=None, inject=None, has\_remote\_tmp=False, persist\_files=False, complex\_args=None) and change the calls to these functions so that has\_remote\_tmp flows along the chain of calls. I hope I am not flooding you with these considerations but I don’t want to start changing prototypes of these things without your approval ! Tell me when you have tried the branch ! Jerome ---
**Author:** ![Michael\_DeHaan](https://avatars.discourse-cdn.com/v4/letter/m/c4cdca/32.png) [@Michael\_DeHaan](https://forum.ansible.com/u/Michael_DeHaan)\ **Post date:** [December 1, 2013, 9:40pm UTC](https://forum.ansible.com/t/ssh-performance-follow-up/14028/23 "2013-12-01T21:40:14Z")
Yeah we want to be careful about changing prototypes of plugins as users may have written their own that are not in core. I believe you can assume you can optimize if: (A) the module is not an action plugin itself (B) the module is a new style python module Thus it seems like this is a hint runner provides when calling the connection. Even still, it should probably utilize kwargs.get(‘persist\_files’, None) to make only the new plugin have to change. Sort of make sense? ---
**Author:** ![Jerome\_Wagner](https://avatars.discourse-cdn.com/v4/letter/j/76d3ee/32.png) [@Jerome\_Wagner](https://forum.ansible.com/u/Jerome_Wagner)\ **Post date:** [December 1, 2013, 11:48pm UTC](https://forum.ansible.com/t/ssh-performance-follow-up/14028/24 "2013-12-01T23:48:15Z")
Ok i understand about not modifying prototypes. I forgot about 3rd party plugins. Can i rely on the fact that tmp==‘’ in the call flow is equivalent to the fact that no remote tmp path was created for the handler ? Regarding NEEDS\_TMPPATH versus a new handler.needs\_remote\_tmp(conn) are you ok for the new function if i call It only when It exists on the handler and default to something like your algo otherwise ? It would seem cleaner to me and i don’t think that the runner gives me the module\_type before the handler was ever called(?) - the decision to create the tmp dir is just before the call to handler.run. I might have the fact that the action called is “normal” to decide that It is a pure module without actions. Do you confirm !=new modules always need a tmp dir because their arguments are sent in a tmp file ? I’ll wait for your answer and prepare a patch. Do you prefer the branch squashed or additional commits ? Jerome [Previous page](https://forum.ansible.com/t/ssh-performance-follow-up/14028.md?page=1)