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

Improve perfomance of node network functions #14

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

tzumainn
Copy link
Collaborator

Performance improved by retrieving a neutron port directory and avoiding later API calls; and by parallelizing port forwarding queries.

Performance improved by retrieving a neutron port directory and
avoiding later API calls; and by parallelizing port forwarding
queries.
@@ -85,22 +98,19 @@ def get_networks_from_port(connection, port, networks_dict={}, floating_ips_dict
parent_network = connection.network.get_network(network=port.network_id)

if port.trunk_details:
with concurrent.futures.ThreadPoolExecutor() as executor:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the concurrent.futures.ThreadPoolExecutor() need to be removed? Does it lead to slower execution with ThreadPoolExecutor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, simply because it should no longer be necessary. The port dictionary should contain all the ports the user can see, so the API query should never be made. The if/else is still there mostly as a just-in-case thing - similar to what the code does with networks - but uncluttering the code feels like a win to me.

Copy link
Collaborator

@DanNiESh DanNiESh left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@tzumainn tzumainn merged commit c7a57fa into CCI-MOC:main Nov 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants