Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-use logic from StrategyBase._load_included_file in StrategyModule.run for free and linear #36470

Merged
merged 1 commit into from
Feb 21, 2018

Conversation

sivel
Copy link
Member

@sivel sivel commented Feb 20, 2018

SUMMARY

Re-use logic from StrategyBase._load_included_file in StrategyModule.run for free and linear. This is the magic incantation that uses less memory and allows higher recursion for include_tasks and with much faster execution.

Before this change, with the below reproducer, ansible would fail with a recursion error at about 30 recursive include_role calls, with an average of 0.86s per include_role.

After this, I was able to get 197, with an average of 0.38s per include_role.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/plugins/strategy

ANSIBLE VERSION
2.4
2.5
2.6
ADDITIONAL INFORMATION

Reproducer:

playbook.yml

---
- hosts: localhost
  gather_facts: false
  vars:
    counter: 0
  tasks:
    - include_role:
        name: recursion

roles/recursion/tasks/main.yml

---
- debug:
    var: counter|int

- set_fact:
    counter: "{{ counter|int + 1 }}"

- include_role:
    name: recursion

Before (exeuction time=0m25.695s):

TASK [recursion : set_fact] ******************************************************************************************************************************************************************************************************************
ok: [localhost] => {"ansible_facts": {"counter": "30"}, "changed": false, "failed": false}

TASK [recursion : include_role] **************************************************************************************************************************************************************************************************************
ERROR! Unexpected Exception, this is probably a bug: maximum recursion depth exceeded
to see the full traceback, use -vvv

After (execution time=1m15.317s):

TASK [recursion : set_fact] ******************************************************************************************************************************************************************************************************************
ok: [localhost] => {"ansible_facts": {"counter": "197"}, "changed": false, "failed": false}

TASK [recursion : include_role] **************************************************************************************************************************************************************************************************************
ERROR! Unexpected Exception, this is probably a bug: maximum recursion depth exceeded
to see the full traceback, use -vvv

@ansibot ansibot added bugfix_pull_request needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Feb 20, 2018
@sivel
Copy link
Member Author

sivel commented Feb 21, 2018

This also resolves the reproducer in #23609 (comment)

@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Feb 21, 2018
…run for free and linear

This improves include_role performance and recursion limits
@sivel sivel changed the title Duplicate logic from StrategyBase._load_included_file into StrategyModule.run for free and linear Re-use logic from StrategyBase._load_included_file in StrategyModule.run for free and linear Feb 21, 2018
@sivel sivel requested a review from jimi-c February 21, 2018 02:33
Copy link
Member

@jimi-c jimi-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sivel sivel merged commit 10fefc7 into ansible:devel Feb 21, 2018
sivel added a commit to sivel/ansible that referenced this pull request Feb 21, 2018
…run for free and linear (ansible#36470)

This improves include_role performance and recursion limits

(cherry picked from commit 10fefc7)
sivel added a commit to sivel/ansible that referenced this pull request Feb 21, 2018
…run for free and linear (ansible#36470)

This improves include_role performance and recursion limits

(cherry picked from commit 10fefc7)
sivel added a commit that referenced this pull request Feb 21, 2018
* Re-use logic from StrategyBase._load_included_file in StrategyModule.run for free and linear (#36470)

This improves include_role performance and recursion limits

(cherry picked from commit 10fefc7)

* Add changelog for 36470
nitzmahone pushed a commit that referenced this pull request Feb 21, 2018
* Re-use logic from StrategyBase._load_included_file in StrategyModule.run for free and linear (#36470)

This improves include_role performance and recursion limits

(cherry picked from commit 10fefc7)

* Add changelog for 36470
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
sivel added a commit that referenced this pull request Mar 20, 2018
sivel added a commit to sivel/ansible that referenced this pull request Mar 20, 2018
sivel added a commit that referenced this pull request Mar 28, 2018
* Fix py3 issue in wait_for_connection (#37646)

(cherry picked from commit 6c3e565)

* Add changelog entry for #36470
abadger pushed a commit that referenced this pull request Mar 28, 2018
sivel added a commit to sivel/ansible that referenced this pull request Apr 17, 2018
sivel added a commit that referenced this pull request Apr 20, 2018
* Error if docker and docker-py are simultaneously (#38884)

* Error if docker and docker-py are simultaneously installed over top of each other. Fixes #36125

* Remove duplicate installed

(cherry picked from commit 68e3ff8)

* Add changelog for #36470
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants