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

Create egress firewall with one db transaction #3847

Merged
merged 1 commit into from Aug 23, 2023

Conversation

npinaeva
Copy link
Member

@npinaeva npinaeva commented Aug 21, 2023

This should improve the time it takes to create an egressfirewall.
Transaction size is limited to 8000 acls, which should be fine based on https://github.com/ovn-org/ovn-kubernetes/blob/master/go-controller/pkg/ovn/external_ids_syncer/acl/acl_sync.go#L58-L60
but should be verified for older version is case of backporting

To test, use 8000-rule egressfirewall https://gist.github.com/npinaeva/7af14f6aa3234694c6d0e082e6c58d14 and find ovnkube-master or ovnkube-controller log Creating *v1.EgressFirewall <namespace>/default took: <n>s

in local testing, before change the result is 31.372159307s, after change 2.599303302s (with db logging enabled, which takes extra time)

@npinaeva npinaeva force-pushed the egress-firewall-perf branch 2 times, most recently from 689e81d to 5aa4f39 Compare August 22, 2023 09:13
Update cleanup test to have dns-based address set as a
leftover, since all acls are commited in one transaction now.

Signed-off-by: Nadia Pinaeva <npinaeva@redhat.com>
Copy link
Contributor

@trozet trozet left a comment

Choose a reason for hiding this comment

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

Nice improvement. Thanks @npinaeva

@trozet trozet merged commit 3265e0b into ovn-org:master Aug 23, 2023
28 checks passed
@npinaeva npinaeva deleted the egress-firewall-perf branch August 23, 2023 18:55
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.

None yet

2 participants