spidernet-io / spiderpool

Underlay and RDMA network solution of the Kubernetes, for bare metal, VM and any public cloud
https://spidernet-io.github.io/spiderpool/
Apache License 2.0
505 stars 72 forks source link

Fix: it is so slow to create a Pod requiring IP from a super big CIDR #3583

Closed lou-lan closed 2 weeks ago

lou-lan commented 3 weeks ago
- release/bug

Description

The function IsDiffIPSet is used to compare ipSourceList and ipExcludeList:

func IsDiffIPSet(ipSourceList, ipExcludeList []net.IP) bool

The DiffIPSet function was previously converting the IP list (first argument) to a map, using string() for the keys. This caused performance issues because:

  1. Converting IPs to strings using string() is slow.
  2. Converting map keys back from strings to net.IP also consumes time.
  3. As the size of ipSourceList increases, the map grows larger, and the time required for map insertions and deletions becomes significant.

Changes

  1. The updated code now uses ipExcludeList for in-place conversion, reducing the use of map.
  2. For Pod IP allocation, the new function FindAvailableIPs is called to find and return an available IP immediately, speeding up the IP allocation process.
func FindAvailableIPs(ipRanges []string, ipList []net.IP, count int) []net.IP

Test

Type Mask Nums Before After
IPv4 /10 4,194,301 Timeout (exceeded 30s) IPAM allocation duration: 0.023414753
IPv6 /106 4,194,301 Timeout (exceeded 30s) IPAM allocation duration: 0.030805433

截屏2024-06-18 16 42 53

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 72.41379% with 16 lines in your changes missing coverage. Please review.

Project coverage is 81.16%. Comparing base (cd1badc) to head (ba95bce).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3583/graphs/tree.svg?width=650&height=150&src=pr&token=YKXY2E4Q8G&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io)](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3583?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) ```diff @@ Coverage Diff @@ ## main #3583 +/- ## ========================================== - Coverage 81.27% 81.16% -0.11% ========================================== Files 50 50 Lines 4352 4391 +39 ========================================== + Hits 3537 3564 +27 - Misses 662 670 +8 - Partials 153 157 +4 ``` | [Flag](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3583/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) | `81.16% <72.41%> (-0.11%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3583?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) | Coverage Δ | | |---|---|---| | [pkg/ippoolmanager/ippool\_manager.go](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3583?src=pr&el=tree&filepath=pkg%2Fippoolmanager%2Fippool_manager.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io#diff-cGtnL2lwcG9vbG1hbmFnZXIvaXBwb29sX21hbmFnZXIuZ28=) | `89.28% <100.00%> (ø)` | | | [pkg/subnetmanager/subnet\_validate.go](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3583?src=pr&el=tree&filepath=pkg%2Fsubnetmanager%2Fsubnet_validate.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io#diff-cGtnL3N1Ym5ldG1hbmFnZXIvc3VibmV0X3ZhbGlkYXRlLmdv) | `92.92% <100.00%> (-0.04%)` | :arrow_down: | | [pkg/subnetmanager/subnet\_manager.go](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3583?src=pr&el=tree&filepath=pkg%2Fsubnetmanager%2Fsubnet_manager.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io#diff-cGtnL3N1Ym5ldG1hbmFnZXIvc3VibmV0X21hbmFnZXIuZ28=) | `41.50% <0.00%> (ø)` | | | [pkg/ip/ip.go](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3583?src=pr&el=tree&filepath=pkg%2Fip%2Fip.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io#diff-cGtnL2lwL2lwLmdv) | `86.39% <72.22%> (-6.13%)` | :arrow_down: |
weizhoublue commented 3 weeks ago

add the issue https://github.com/spidernet-io/spiderpool/issues/3541

weizhoublue commented 3 weeks ago

add test for ipv6

Icarus9913 commented 3 weeks ago

Nice fix :+1::+1::+1:

weizhoublue commented 3 weeks ago

please change more meaningless references of ip.IPsDiffSet to IsDiffIPSet please find more of it

spidersubnet.go

                        diffIps := ip.IPsDiffSet(ips1, ips2, false)
                        if diffIps != nil {
                            GinkgoWriter.Printf("inconsistent ip records in subnet %v/%v and pool %v/%v \n", subnetName, ips2, pool.Name, ips1)
                            continue LOOP
                        }

subnet_validate.go

            diffSet := spiderpoolip.IPsDiffSet(poolIPs, subnetIPs, false)
            if len(diffSet) != 0 {
                return field.Invalid(ipsField, subnet.Spec.IPs, fmt.Sprintf("SpiderIPPool '%s' owns some IP addresses that SpiderSubnet '%s' can't control", tmpPool.Name, subnet.Name))
            }
ty-dc commented 2 weeks ago

We have made some changes to the CI upgrade. If it fails now, please rebase main and try again.