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

Performance degrades severely under prefix load #146

Open
bplotnick opened this issue May 8, 2024 · 2 comments
Open

Performance degrades severely under prefix load #146

bplotnick opened this issue May 8, 2024 · 2 comments

Comments

@bplotnick
Copy link

When a prefix is "loaded" - meaning that we have acquired a large number of ips from it - performance degrades as O(number of acquired IPs)

The benchmark code misses this because it immediately releases IPs after acquiring them. But with a slight change, you can see this:

--- a/prefix_benchmark_test.go
+++ b/prefix_benchmark_test.go
@@ -33,6 +33,7 @@ func BenchmarkAcquireIP(b *testing.B) {
                if err != nil {
                        panic(err)
                }
+               ips := make([]*IP, b.N)
                for n := 0; n < b.N; n++ {
                        ip, err := ipam.AcquireIP(ctx, p.Cidr)
                        if err != nil {
@@ -41,7 +42,10 @@ func BenchmarkAcquireIP(b *testing.B) {
                        if ip == nil {
                                panic("IP nil")
                        }
-                       p, err = ipam.ReleaseIP(ctx, ip)
+                       ips[n] = ip
+               }
+               for _, i := range ips {
+                       _, err := ipam.ReleaseIP(ctx, i)
                        if err != nil {
                                panic(err)
                        }

You should see performance degrade significantly.

On my machine, if i measure the time it takes to acquire the last IP, with in-memory storage the 2nd acquired is ~1us, the 101st is 13us, and the 10001 is 1ms.

I believe this is due to this loop: https://github.com/metal-stack/go-ipam/blob/master/prefix.go#L378, which will scan the entire range.

@majst01
Copy link
Contributor

majst01 commented May 9, 2024

Interesting observation, your guess where the time is spent might be correct, but i have actually no better solution in mind howto find the next available ip in the prefix, other than iterating through the prefix.

First action would be to write a benchmark for this specific func acquireSpecificIPInternal and see if your guess is correct.
If you have time to write a PR for that, i am happy to review

@majst01
Copy link
Contributor

majst01 commented May 11, 2024

@bplotnick added a benchmark which shows the root cause in this #147 also made the memory case at least twice as fast

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

No branches or pull requests

2 participants