Decouple fadeIn from loadbalancer

[RomanZavodskikh]

Decouple fadeIn from loadbalancer

[RomanZavodskikh]

Those test were moved from loadbalancer/fadein_test.go

[szuecs]

lgtm

[szuecs]

👍

[szuecs]

👍

[AlexanderYastrebov]

Note: skipFadingEndpoints did linear search over notFadingIndexes and this would do linear search over LBContext.LBEndpoints (potentially having fading endpoints filtered out) so it should not be worse performance-wise.

[RomanZavodskikh]

I tried having an LBEndpointsSet together with LBEndpoint slice and using it in SkipEndpoint, however, this using does not improve performance at all:

goos: linux
goarch: amd64
pkg: github.com/zalando/skipper/proxy
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
                                                     │ decoupleFadeIn.txt │       decoupleFadeInWithSet.txt        │
                                                     │       sec/op       │    sec/op      vs base                 │
FadeIn/power-of-n-random-choices,_11,_1_clients-16            35.40µ ± 2%    72.97µ ±  1%  +106.16% (p=0.000 n=15)
FadeIn/power-of-n-random-choices,_11,_4_clients-16            14.01µ ± 5%    28.01µ ±  9%  +100.02% (p=0.000 n=15)
FadeIn/power-of-n-random-choices,_11,_16_clients-16           4.744µ ± 4%   16.192µ ±  2%  +241.32% (p=0.000 n=15)
FadeIn/power-of-n-random-choices,_11,_64_clients-16           4.474µ ± 3%   12.780µ ±  1%  +185.65% (p=0.000 n=15)
FadeIn/power-of-n-random-choices,_11,_256_clients-16          4.003µ ± 2%    9.654µ ±  2%  +141.17% (p=0.000 n=15)
FadeIn/random,_11,_1_clients-16                               36.58µ ± 2%    77.11µ ±  1%  +110.79% (p=0.000 n=15)
FadeIn/random,_11,_4_clients-16                               13.73µ ± 3%    26.47µ ± 13%   +92.83% (p=0.000 n=15)
FadeIn/random,_11,_16_clients-16                              4.680µ ± 7%   16.234µ ±  1%  +246.88% (p=0.000 n=15)
FadeIn/random,_11,_64_clients-16                              4.617µ ± 2%   13.017µ ±  3%  +181.94% (p=0.000 n=15)
FadeIn/random,_11,_256_clients-16                             4.032µ ± 2%   10.251µ ±  5%  +154.24% (p=0.000 n=15)
FadeIn/round-robin,_11,_1_clients-16                          36.45µ ± 2%    76.75µ ±  1%  +110.56% (p=0.000 n=15)
FadeIn/round-robin,_11,_4_clients-16                          12.58µ ± 4%    27.09µ ±  4%  +115.37% (p=0.000 n=15)
FadeIn/round-robin,_11,_16_clients-16                         4.714µ ± 5%   17.271µ ±  3%  +266.38% (p=0.000 n=15)
FadeIn/round-robin,_11,_64_clients-16                         4.583µ ± 3%   14.322µ ±  5%  +212.50% (p=0.000 n=15)
FadeIn/round-robin,_11,_256_clients-16                        4.175µ ± 2%   10.887µ ±  9%  +160.77% (p=0.000 n=15)
FadeIn/consistent-hash,_11,_1_clients-16                      36.05µ ± 2%    81.63µ ±  3%  +126.43% (p=0.000 n=15)
FadeIn/consistent-hash,_11,_4_clients-16                      12.47µ ± 7%    30.50µ ± 10%  +144.55% (p=0.000 n=15)
FadeIn/consistent-hash,_11,_16_clients-16                     4.402µ ± 2%   16.419µ ±  4%  +272.99% (p=0.000 n=15)
FadeIn/consistent-hash,_11,_64_clients-16                     4.300µ ± 2%   13.765µ ±  1%  +220.12% (p=0.000 n=15)
FadeIn/consistent-hash,_11,_256_clients-16                    3.974µ ± 1%   11.031µ ±  4%  +177.58% (p=0.000 n=15)
geomean                                                       8.328µ         21.87µ        +162.62%

[szuecs]

The name sounds a bit weird.
Maybe: noFadeIn or shouldFadeIn

or drop the func and change the logic below ( filterFadeIn ) to:

if rt.BackendType != eskip.LBBackend || rt.LBFadeInDuration <= 0 { return endpoints }

[AlexanderYastrebov]

Do we need to check backend type at all? This code is only called for loadbalanced backends anyway as it passes endpoints

[AlexanderYastrebov]

👍

[RomanZavodskikh]

👍

[RomanZavodskikh]

All comments that are left are about code refactoring, feature-wise everything seems good.

[RomanZavodskikh]

Loadtested v0.19.32, loadtests are okay.