Skip to content

Conversation

squeed
Copy link
Contributor

@squeed squeed commented Oct 23, 2024

Searching for conflicts when a given prefixes has a lot of resources is very expensive. Specifically, all resources must be sorted before checking for conflicts.

Given that most resources do not set metadata that can conflict, this is not necessary. So, skip searching for conflicts if the supplied resource cannot cause a conflict.

Fixes slow policy import times when many network policies reference the same CIDR.

…rces

This mirrors a real-world use-case where many policies reference the
same CIDR.

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
@squeed squeed added kind/performance There is a performance impact of this. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. affects/v1.14 This issue affects v1.14 branch affects/v1.15 This issue affects v1.15 branch affects/v1.16 This issue affects v1.16 branch labels Oct 23, 2024
@squeed squeed requested a review from a team as a code owner October 23, 2024 11:16
@squeed squeed requested a review from doniacld October 23, 2024 11:16
@squeed squeed force-pushed the fix-lots-of-resources branch from aadd577 to c3f4600 Compare October 23, 2024 12:53
@squeed
Copy link
Contributor Author

squeed commented Oct 23, 2024

/test

Searching for conflicts when a given prefixes has a lot of resources is
very expensive. Specifically, all resources must be sorted before
checking for conflicts.

Given that most resources do not set metadata that can conflict, this is
not necessary. So, skip searching for conflicts if the supplied resource
cannot cause a conflict.

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
@squeed squeed force-pushed the fix-lots-of-resources branch from c3f4600 to f95080c Compare October 23, 2024 14:32
@squeed
Copy link
Contributor Author

squeed commented Oct 23, 2024

/test

Copy link
Contributor

@doniacld doniacld left a comment

Choose a reason for hiding this comment

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

LGTM!

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 24, 2024
@squeed squeed added this pull request to the merge queue Oct 24, 2024
Merged via the queue into cilium:main with commit d31f7dd Oct 24, 2024
62 of 63 checks passed
@squeed squeed deleted the fix-lots-of-resources branch October 24, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/v1.14 This issue affects v1.14 branch affects/v1.15 This issue affects v1.15 branch affects/v1.16 This issue affects v1.16 branch kind/performance There is a performance impact of this. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants