Skip to content

Commit

Permalink
refactor: remove EnablePassiveHealthCheck flag
Browse files Browse the repository at this point in the history 
rely on PassiveHealthCheck struct pointer to be nil or not nil instead

Signed-off-by: Roman Zavodskikh <[email protected]>
  • Loading branch information
Roman Zavodskikh committed Nov 25, 2024
1 parent f6f9efb commit 6e5f9d5
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 188 deletions.
2 changes: 1 addition & 1 deletion VERSION
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v0.21
v0.22
84 changes: 40 additions & 44 deletions proxy/healthy_endpoints_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ const (
period = 100 * time.Millisecond
)

func defaultEndpointRegistry() *routing.EndpointRegistry {
return routing.NewEndpointRegistry(routing.RegistryOptions{
PassiveHealthCheckEnabled: true,
StatsResetPeriod: period,
MinRequests: 2,
MaxHealthCheckDropProbability: 0.95,
MinHealthCheckDropProbability: 0.01,
})
func defaultPassiveHealthCheck() *routing.PassiveHealthCheck {
return &routing.PassiveHealthCheck{
Period: period,
MinRequests: 2,
MaxDropProbability: 0.95,
MinDropProbability: 0.01,
MaxUnhealthyEndpointsRatio: 1.0,
}
}

func sendGetRequest(t *testing.T, ps *httptest.Server, consistentHashKey int) *http.Response {
Expand Down Expand Up @@ -62,14 +62,10 @@ func fireVegeta(t *testing.T, ps *httptest.Server, freq int, per time.Duration,

func setupProxy(t *testing.T, doc string) (*metricstest.MockMetrics, *httptest.Server) {
m := &metricstest.MockMetrics{}
endpointRegistry := defaultEndpointRegistry()
endpointRegistry := routing.NewEndpointRegistry(routing.RegistryOptions{PassiveHealthCheck: defaultPassiveHealthCheck()})
proxyParams := Params{
EnablePassiveHealthCheck: true,
EndpointRegistry: endpointRegistry,
Metrics: m,
PassiveHealthCheck: &PassiveHealthCheck{
MaxUnhealthyEndpointsRatio: 1.0,
},
EndpointRegistry: endpointRegistry,
Metrics: m,
}

return m, setupProxyWithCustomProxyParams(t, doc, proxyParams)
Expand All @@ -78,12 +74,8 @@ func setupProxy(t *testing.T, doc string) (*metricstest.MockMetrics, *httptest.S
func setupProxyWithCustomEndpointRegisty(t *testing.T, doc string, endpointRegistry *routing.EndpointRegistry) (*metricstest.MockMetrics, *httptest.Server) {
m := &metricstest.MockMetrics{}
proxyParams := Params{
EnablePassiveHealthCheck: true,
EndpointRegistry: endpointRegistry,
Metrics: m,
PassiveHealthCheck: &PassiveHealthCheck{
MaxUnhealthyEndpointsRatio: 1.0,
},
EndpointRegistry: endpointRegistry,
Metrics: m,
}

return m, setupProxyWithCustomProxyParams(t, doc, proxyParams)
Expand Down Expand Up @@ -151,15 +143,18 @@ func TestPHCWithoutRequests(t *testing.T) {

func TestPHCForSingleHealthyEndpoint(t *testing.T) {
servicesString := setupServices(t, 1, 0)
endpointRegistry := defaultEndpointRegistry()
endpointRegistry := routing.NewEndpointRegistry(routing.RegistryOptions{
PassiveHealthCheck: &routing.PassiveHealthCheck{
MaxUnhealthyEndpointsRatio: 1.0,
MaxDropProbability: 0.95,
MinRequests: 2,
Period: period,
},
})

doc := fmt.Sprintf(`* -> %s`, servicesString)
tp, err := newTestProxyWithParams(doc, Params{
EnablePassiveHealthCheck: true,
EndpointRegistry: endpointRegistry,
PassiveHealthCheck: &PassiveHealthCheck{
MaxUnhealthyEndpointsRatio: 1.0,
},
EndpointRegistry: endpointRegistry,
})
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -227,11 +222,13 @@ func TestPHC(t *testing.T) {

t.Run("consistentHash", func(t *testing.T) {
consistantHashCustomEndpointRegistry := routing.NewEndpointRegistry(routing.RegistryOptions{
PassiveHealthCheckEnabled: true,
StatsResetPeriod: 1 * time.Second,
MinRequests: 1, // with 2 test case fails on github actions
MaxHealthCheckDropProbability: 0.95,
MinHealthCheckDropProbability: 0.01,
PassiveHealthCheck: &routing.PassiveHealthCheck{
Period: 1 * time.Second,
MinRequests: 1, // with 2 test case fails on github actions
MaxDropProbability: 0.95,
MinDropProbability: 0.01,
MaxUnhealthyEndpointsRatio: 1.0,
},
})
mockMetrics, ps := setupProxyWithCustomEndpointRegisty(t, fmt.Sprintf(`* -> backendTimeout("5ms") -> consistentHashKey("${request.header.ConsistentHashKey}") -> <consistentHash, %s>`,
servicesString), consistantHashCustomEndpointRegistry)
Expand All @@ -245,11 +242,13 @@ func TestPHC(t *testing.T) {

t.Run("consistent hash with balance factor", func(t *testing.T) {
consistantHashCustomEndpointRegistry := routing.NewEndpointRegistry(routing.RegistryOptions{
PassiveHealthCheckEnabled: true,
StatsResetPeriod: 1 * time.Second,
MinRequests: 1, // with 2 test case fails on github actions
MaxHealthCheckDropProbability: 0.95,
MinHealthCheckDropProbability: 0.01,
PassiveHealthCheck: &routing.PassiveHealthCheck{
Period: 1 * time.Second,
MinRequests: 1, // with 2 test case fails on github actions
MaxDropProbability: 0.95,
MinDropProbability: 0.01,
MaxUnhealthyEndpointsRatio: 1.0,
},
})
mockMetrics, ps := setupProxyWithCustomEndpointRegisty(t, fmt.Sprintf(`* -> backendTimeout("5ms") -> consistentHashKey("${request.header.ConsistentHashKey}") -> consistentHashBalanceFactor(1.25) -> <consistentHash, %s>`,
servicesString), consistantHashCustomEndpointRegistry)
Expand Down Expand Up @@ -301,14 +300,11 @@ func TestPHCMaxUnhealthyEndpointsRatioParam(t *testing.T) {

servicesString := setupServices(t, healthy, unhealthy)
mockMetrics := &metricstest.MockMetrics{}
endpointRegistry := defaultEndpointRegistry()
passiveHealthCheck := defaultPassiveHealthCheck()
passiveHealthCheck.MaxUnhealthyEndpointsRatio = maxUnhealthyEndpointsRatio
proxyParams := Params{
EnablePassiveHealthCheck: true,
EndpointRegistry: endpointRegistry,
Metrics: mockMetrics,
PassiveHealthCheck: &PassiveHealthCheck{
MaxUnhealthyEndpointsRatio: maxUnhealthyEndpointsRatio,
},
EndpointRegistry: routing.NewEndpointRegistry(routing.RegistryOptions{PassiveHealthCheck: passiveHealthCheck}),
Metrics: mockMetrics,
}
ps := setupProxyWithCustomProxyParams(t, fmt.Sprintf(`* -> backendTimeout("20ms") -> <random, %s>`,
servicesString), proxyParams)
Expand Down
66 changes: 19 additions & 47 deletions proxy/proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,34 +146,12 @@ type OpenTracingParams struct {
ExcludeTags []string
}

type PassiveHealthCheck struct {
// The period of time after which the endpointregistry begins to calculate endpoints statistics
// from scratch
Period time.Duration

// The minimum number of total requests that should be sent to an endpoint in a single period to
// potentially opt out the endpoint from the list of healthy endpoints
MinRequests int64

// The minimal ratio of failed requests in a single period to potentially opt out the endpoint
// from the list of healthy endpoints. This ratio is equal to the minimal non-zero probability of
// dropping endpoint out from load balancing for every specific request
MinDropProbability float64

// The maximum probability of unhealthy endpoint to be dropped out from load balancing for every specific request
MaxDropProbability float64

// MaxUnhealthyEndpointsRatio is the maximum ratio of unhealthy endpoints in the list of all endpoints PHC will check
// in case of all endpoints are unhealthy
MaxUnhealthyEndpointsRatio float64
}

func InitPassiveHealthChecker(o map[string]string) (bool, *PassiveHealthCheck, error) {
func InitPassiveHealthChecker(o map[string]string) (*routing.PassiveHealthCheck, error) {
if len(o) == 0 {
return false, &PassiveHealthCheck{}, nil
return nil, nil
}

result := &PassiveHealthCheck{
result := &routing.PassiveHealthCheck{
MinDropProbability: 0.0,
MaxUnhealthyEndpointsRatio: 1.0,
}
Expand All @@ -190,62 +168,62 @@ func InitPassiveHealthChecker(o map[string]string) (bool, *PassiveHealthCheck, e
case "period":
period, err := time.ParseDuration(value)
if err != nil {
return false, nil, fmt.Errorf("passive health check: invalid period value: %s", value)
return nil, fmt.Errorf("passive health check: invalid period value: %s", value)
}
if period < 0 {
return false, nil, fmt.Errorf("passive health check: invalid period value: %s", value)
return nil, fmt.Errorf("passive health check: invalid period value: %s", value)
}
result.Period = period
case "min-requests":
minRequests, err := strconv.Atoi(value)
if err != nil {
return false, nil, fmt.Errorf("passive health check: invalid minRequests value: %s", value)
return nil, fmt.Errorf("passive health check: invalid minRequests value: %s", value)
}
if minRequests < 0 {
return false, nil, fmt.Errorf("passive health check: invalid minRequests value: %s", value)
return nil, fmt.Errorf("passive health check: invalid minRequests value: %s", value)
}
result.MinRequests = int64(minRequests)
case "max-drop-probability":
maxDropProbability, err := strconv.ParseFloat(value, 64)
if err != nil {
return false, nil, fmt.Errorf("passive health check: invalid maxDropProbability value: %s", value)
return nil, fmt.Errorf("passive health check: invalid maxDropProbability value: %s", value)
}
if maxDropProbability < 0 || maxDropProbability > 1 {
return false, nil, fmt.Errorf("passive health check: invalid maxDropProbability value: %s", value)
return nil, fmt.Errorf("passive health check: invalid maxDropProbability value: %s", value)
}
result.MaxDropProbability = maxDropProbability

/* optional parameters */
case "min-drop-probability":
minDropProbability, err := strconv.ParseFloat(value, 64)
if err != nil {
return false, nil, fmt.Errorf("passive health check: invalid minDropProbability value: %s", value)
return nil, fmt.Errorf("passive health check: invalid minDropProbability value: %s", value)
}
if minDropProbability < 0 || minDropProbability > 1 {
return false, nil, fmt.Errorf("passive health check: invalid minDropProbability value: %s", value)
return nil, fmt.Errorf("passive health check: invalid minDropProbability value: %s", value)
}
result.MinDropProbability = minDropProbability
case "max-unhealthy-endpoints-ratio":
maxUnhealthyEndpointsRatio, err := strconv.ParseFloat(value, 64)
if err != nil {
return false, nil, fmt.Errorf("passive health check: invalid maxUnhealthyEndpointsRatio value: %q", value)
return nil, fmt.Errorf("passive health check: invalid maxUnhealthyEndpointsRatio value: %q", value)
}
if maxUnhealthyEndpointsRatio < 0 || maxUnhealthyEndpointsRatio > 1 {
return false, nil, fmt.Errorf("passive health check: invalid maxUnhealthyEndpointsRatio value: %q", value)
return nil, fmt.Errorf("passive health check: invalid maxUnhealthyEndpointsRatio value: %q", value)
}
result.MaxUnhealthyEndpointsRatio = maxUnhealthyEndpointsRatio
default:
return false, nil, fmt.Errorf("passive health check: invalid parameter: key=%s,value=%s", key, value)
return nil, fmt.Errorf("passive health check: invalid parameter: key=%s,value=%s", key, value)
}
}

if len(requiredParams) != 0 {
return false, nil, fmt.Errorf("passive health check: missing required parameters %+v", maps.Keys(requiredParams))
return nil, fmt.Errorf("passive health check: missing required parameters %+v", maps.Keys(requiredParams))
}
if result.MinDropProbability >= result.MaxDropProbability {
return false, nil, fmt.Errorf("passive health check: minDropProbability should be less than maxDropProbability")
return nil, fmt.Errorf("passive health check: minDropProbability should be less than maxDropProbability")
}
return true, result, nil
return result, nil
}

// Proxy initialization options.
Expand Down Expand Up @@ -354,12 +332,6 @@ type Params struct {
// and returns some metadata about endpoint. Information about the metadata
// returned from the registry could be found in routing.Metrics interface.
EndpointRegistry *routing.EndpointRegistry

// EnablePassiveHealthCheck enables the healthy endpoints checker
EnablePassiveHealthCheck bool

// PassiveHealthCheck defines the parameters for the healthy endpoints checker.
PassiveHealthCheck *PassiveHealthCheck
}

type (
Expand Down Expand Up @@ -838,10 +810,10 @@ func WithParams(p Params) *Proxy {
hostname := os.Getenv("HOSTNAME")

var healthyEndpointsChooser *healthyEndpoints
if p.EnablePassiveHealthCheck {
if p.EndpointRegistry.GetPassiveHealthCheck() != nil {
healthyEndpointsChooser = &healthyEndpoints{
rnd: rand.New(loadbalancer.NewLockedSource()),
maxUnhealthyEndpointsRatio: p.PassiveHealthCheck.MaxUnhealthyEndpointsRatio,
maxUnhealthyEndpointsRatio: p.EndpointRegistry.GetPassiveHealthCheck().MaxUnhealthyEndpointsRatio,
}
}
return &Proxy{
Expand Down
Loading

0 comments on commit 6e5f9d5

Please sign in to comment.