Skip to content

Commit 5968d85

Browse files
hoffmaenhoffmaen
committed
refactor: Skip metric reporting for keepalive registrations without algorithm change
Adjust the subscriber to preserve the distinction between an absent load balancing algorithm field (nil, no change intended) and an explicitly empty field (algorithm removed, revert to platform default). This allows reportEndpointsPerPool to early-return when the endpoint carries no algorithm, avoiding unnecessary UncaptureEndpointsPerPool calls on every keepalive registration for non-HB routes.
1 parent 9746a83 commit 5968d85

4 files changed

Lines changed: 63 additions & 18 deletions

File tree

‎src/code.cloudfoundry.org/gorouter/mbus/subscriber.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type RegistryMessage struct {
4141
}
4242

4343
type RegistryMessageOpts struct {
44-
LoadBalancingAlgorithm string `json:"loadbalancing"`
44+
LoadBalancingAlgorithm *string `json:"loadbalancing"`
4545
HashHeaderName string `json:"hash_header"`
4646
HashBalance float64 `json:"hash_balance,string"`
4747
}
@@ -61,9 +61,13 @@ func (rm *RegistryMessage) makeEndpoint(http2Enabled bool, globalRoutingAlgo str
6161
protocol = "http1"
6262
}
6363

64-
lbAlgo := globalRoutingAlgo
65-
if rm.Options.LoadBalancingAlgorithm != "" {
66-
lbAlgo = rm.Options.LoadBalancingAlgorithm
64+
lbAlgo := ""
65+
if rm.Options.LoadBalancingAlgorithm != nil {
66+
if *rm.Options.LoadBalancingAlgorithm != "" {
67+
lbAlgo = *rm.Options.LoadBalancingAlgorithm
68+
} else {
69+
lbAlgo = globalRoutingAlgo
70+
}
6771
}
6872

6973
return route.NewEndpoint(&route.EndpointOpts{

‎src/code.cloudfoundry.org/gorouter/mbus/subscriber_test.go‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ var _ = Describe("Subscriber", func() {
566566
App: "app",
567567
Protocol: "http2",
568568
Uris: []route.Uri{"test.example.com"},
569-
Options: mbus.RegistryMessageOpts{LoadBalancingAlgorithm: expectedLBAlgo},
569+
Options: mbus.RegistryMessageOpts{LoadBalancingAlgorithm: &expectedLBAlgo},
570570
}
571571
data, err := json.Marshal(msg)
572572
Expect(err).NotTo(HaveOccurred())
@@ -594,7 +594,7 @@ var _ = Describe("Subscriber", func() {
594594
Eventually(process.Ready()).Should(BeClosed())
595595
})
596596

597-
It("endpoint is constructed with the global default load balancing algorithm", func() {
597+
It("endpoint is constructed with an empty load balancing algorithm when field is absent", func() {
598598
var msg = mbus.RegistryMessage{
599599
Host: "host",
600600
App: "app",
@@ -607,6 +607,32 @@ var _ = Describe("Subscriber", func() {
607607
err = natsClient.Publish("router.register", data)
608608
Expect(err).ToNot(HaveOccurred())
609609

610+
Eventually(registry.RegisterCallCount).Should(Equal(1))
611+
_, originalEndpoint := registry.RegisterArgsForCall(0)
612+
expectedEndpoint := route.NewEndpoint(&route.EndpointOpts{
613+
Host: "host",
614+
AppId: "app",
615+
Protocol: "http2",
616+
})
617+
618+
Expect(originalEndpoint).To(Equal(expectedEndpoint))
619+
})
620+
621+
It("endpoint gets the global default algorithm when the algorithm is explicitly set to empty string", func() {
622+
emptyStr := ""
623+
var msg = mbus.RegistryMessage{
624+
Host: "host",
625+
App: "app",
626+
Protocol: "http2",
627+
Uris: []route.Uri{"test.example.com"},
628+
Options: mbus.RegistryMessageOpts{LoadBalancingAlgorithm: &emptyStr},
629+
}
630+
data, err := json.Marshal(msg)
631+
Expect(err).NotTo(HaveOccurred())
632+
633+
err = natsClient.Publish("router.register", data)
634+
Expect(err).ToNot(HaveOccurred())
635+
610636
Eventually(registry.RegisterCallCount).Should(Equal(1))
611637
_, originalEndpoint := registry.RegisterArgsForCall(0)
612638
expectedEndpoint := route.NewEndpoint(&route.EndpointOpts{
@@ -635,7 +661,7 @@ var _ = Describe("Subscriber", func() {
635661
Protocol: "http2",
636662
Uris: []route.Uri{"test.example.com"},
637663
Options: mbus.RegistryMessageOpts{
638-
LoadBalancingAlgorithm: expectedLBAlgo,
664+
LoadBalancingAlgorithm: &expectedLBAlgo,
639665
HashHeaderName: "X-Header",
640666
HashBalance: 1.5,
641667
},
@@ -669,7 +695,7 @@ var _ = Describe("Subscriber", func() {
669695
Protocol: "http2",
670696
Uris: []route.Uri{"test.example.com"},
671697
Options: mbus.RegistryMessageOpts{
672-
LoadBalancingAlgorithm: expectedLBAlgo,
698+
LoadBalancingAlgorithm: &expectedLBAlgo,
673699
HashHeaderName: "X-Header",
674700
},
675701
}
@@ -703,7 +729,7 @@ var _ = Describe("Subscriber", func() {
703729
Protocol: "http2",
704730
Uris: []route.Uri{"test.example.com"},
705731
Options: mbus.RegistryMessageOpts{
706-
LoadBalancingAlgorithm: expectedLBAlgo,
732+
LoadBalancingAlgorithm: &expectedLBAlgo,
707733
HashHeaderName: "X-Header",
708734
},
709735
}

‎src/code.cloudfoundry.org/gorouter/registry/registry.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -475,12 +475,15 @@ func (r *RouteRegistry) freshenRoutes() {
475475
}
476476

477477
// reportEndpointsPerPool reports the endpoints_per_pool metric for hash-based (HB) route pools.
478-
// For non-HB endpoints, it unconditionally deletes stale metric entries.
478+
// When the endpoint carries no algorithm (empty string), the registration is a keepalive with no
479+
// algorithm change, and metric reporting is skipped entirely.
480+
// For non-HB endpoints with an explicit algorithm, it deletes stale HB metric entries.
479481
func (r *RouteRegistry) reportEndpointsPerPool(uri route.Uri, endpoint *route.Endpoint) {
482+
if endpoint.LoadBalancingAlgorithm == "" {
483+
return
484+
}
485+
480486
if endpoint.LoadBalancingAlgorithm != config.LOAD_BALANCE_HB {
481-
// Delete stale entries for routes that have switched from HB to another load balancing algorithm.
482-
// This is called unconditionally because the pool may still report its algorithm as HB
483-
// even after the HB setting was removed (setPoolLoadBalancingAlgorithm ignores empty values).
484487
r.reporter.UncaptureEndpointsPerPool(string(uri), config.LOAD_BALANCE_HB)
485488
return
486489
}

‎src/code.cloudfoundry.org/gorouter/registry/registry_test.go‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ var _ = Describe("RouteRegistry", func() {
698698
Expect(found).To(BeTrue(), "expected UncaptureEndpointsPerPool to be called for the switched route")
699699
})
700700

701-
It("deletes the HB metric when the HB algorithm is removed and endpoint registers with empty algorithm", func() {
701+
It("deletes the HB metric when the HB algorithm is removed and endpoint registers with the platform default", func() {
702702
hbEndpoint := route.NewEndpoint(&route.EndpointOpts{
703703
Host: "192.168.1.1",
704704
Port: 8080,
@@ -709,16 +709,16 @@ var _ = Describe("RouteRegistry", func() {
709709
r.Register("removed-hb.example.com", hbEndpoint)
710710
Expect(reporter.CaptureEndpointsPerPoolCallCount()).To(BeNumerically(">=", 1))
711711

712-
// Endpoint re-registers with empty LB algorithm (HB setting removed from route)
713-
noAlgoEndpoint := route.NewEndpoint(&route.EndpointOpts{
712+
// Endpoint re-registers with platform default algorithm (HB setting explicitly removed from route)
713+
defaultAlgoEndpoint := route.NewEndpoint(&route.EndpointOpts{
714714
Host: "192.168.1.1",
715715
Port: 8080,
716716
PrivateInstanceId: "id-1",
717-
LoadBalancingAlgorithm: "",
717+
LoadBalancingAlgorithm: config.LOAD_BALANCE_RR,
718718
})
719719

720720
deleteBefore := reporter.UncaptureEndpointsPerPoolCallCount()
721-
r.Register("removed-hb.example.com", noAlgoEndpoint)
721+
r.Register("removed-hb.example.com", defaultAlgoEndpoint)
722722

723723
Expect(reporter.UncaptureEndpointsPerPoolCallCount()).To(BeNumerically(">", deleteBefore))
724724
found := false
@@ -732,6 +732,18 @@ var _ = Describe("RouteRegistry", func() {
732732
Expect(found).To(BeTrue(), "expected UncaptureEndpointsPerPool to be called when HB algorithm is removed")
733733
})
734734

735+
It("skips metric reporting when the endpoint has no algorithm set", func() {
736+
noAlgoEndpoint := route.NewEndpoint(&route.EndpointOpts{
737+
Host: "192.168.1.1",
738+
Port: 8080,
739+
PrivateInstanceId: "id-1",
740+
})
741+
742+
r.Register("no-algo.example.com", noAlgoEndpoint)
743+
Expect(reporter.CaptureEndpointsPerPoolCallCount()).To(Equal(0))
744+
Expect(reporter.UncaptureEndpointsPerPoolCallCount()).To(Equal(0))
745+
})
746+
735747
It("does not change the metric count when an existing HB endpoint is updated", func() {
736748
hbEndpoint := route.NewEndpoint(&route.EndpointOpts{
737749
Host: "192.168.1.1",

0 commit comments

Comments
 (0)