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

Fix: frequent 503 errors when connecting to a Service experiencing high Pod churn #4754

Merged
merged 5 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 46 additions & 5 deletions internal/provider/kubernetes/status_updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import (
"context"
"errors"
"time"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -56,14 +57,25 @@
type UpdateHandler struct {
log logr.Logger
client client.Client
sendUpdates chan struct{}
updateChannel chan Update
writer *UpdateWriter
}

func NewUpdateHandler(log logr.Logger, client client.Client) *UpdateHandler {
sendUpdates := make(chan struct{})
updateChannel := make(chan Update, 100)
return &UpdateHandler{
log: log,
client: client,
updateChannel: make(chan Update, 100),
sendUpdates: sendUpdates,
updateChannel: updateChannel,
writer: &UpdateWriter{
log: log,
enabled: sendUpdates,
updateChannel: updateChannel,
eventsBeforeEnabled: make(chan Update, 1000),
zhaohuabing marked this conversation as resolved.
Show resolved Hide resolved
},
}
}

Expand Down Expand Up @@ -127,6 +139,10 @@
u.log.Info("started status update handler")
defer u.log.Info("stopped status update handler")

// Enable Updaters to start sending updates to this handler.
close(u.sendUpdates)
u.writer.handleEventsReceivedBeforeEnabled()

for {
select {
case <-ctx.Done():
Expand All @@ -142,9 +158,7 @@

// Writer retrieves the interface that should be used to write to the UpdateHandler.
func (u *UpdateHandler) Writer() Updater {
return &UpdateWriter{
updateChannel: u.updateChannel,
}
return u.writer
}

// Updater describes an interface to send status updates somewhere.
Expand All @@ -154,13 +168,40 @@

// UpdateWriter takes status updates and sends these to the UpdateHandler via a channel.
type UpdateWriter struct {
log logr.Logger
enabled <-chan struct{}
updateChannel chan<- Update
// a temporary buffer to store events received before the Updater is enabled.
// These events will be sent to the update channel once the Updater is enabled.
eventsBeforeEnabled chan Update
}

// Send sends the given Update off to the update channel for writing by the UpdateHandler.
func (u *UpdateWriter) Send(update Update) {
// Non-blocking receive to see if we should pass along update.
u.updateChannel <- update
select {
zhaohuabing marked this conversation as resolved.
Show resolved Hide resolved
case <-u.enabled:
u.updateChannel <- update
default:
if len(u.eventsBeforeEnabled) < cap(u.eventsBeforeEnabled) {
u.log.Info("received a status update while disabled, storing for later", "event", update.NamespacedName)
u.eventsBeforeEnabled <- update
} else {
// If the buffer is full, drop the event to avoid blocking the sender.
u.log.Error(errors.New("dropping status update, buffer full"), "event", update.NamespacedName)
}

Check warning on line 192 in internal/provider/kubernetes/status_updater.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status_updater.go#L185-L192

Added lines #L185 - L192 were not covered by tests
}
}

// handleEventsReceivedBeforeEnabled sends the events received before the Updater was enabled to the update channel.
func (u *UpdateWriter) handleEventsReceivedBeforeEnabled() {
go func() {
for e := range u.eventsBeforeEnabled {
u.log.Info("sending stored status update", "event", e.NamespacedName)
u.updateChannel <- e
}
close(u.eventsBeforeEnabled)

Check warning on line 203 in internal/provider/kubernetes/status_updater.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status_updater.go#L200-L203

Added lines #L200 - L203 were not covered by tests
}()
}

// isStatusEqual checks if two objects have equivalent status.
Expand Down
1 change: 1 addition & 0 deletions release-notes/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ bug fixes: |
Fixed failed to update SecurityPolicy resources with the `backendRef` field specified
Fixed Envoy rejecting TCP Listeners that have no attached TCPRoutes
Fixed xDS translation failed when oidc tokenEndpoint and jwt remoteJWKS are specified in the same SecurityPolicy and using the same hostname
Fixed frequent 503 errors when connecting to a Service experiencing high Pod churn

# Enhancements that improve performance.
performance improvements: |
Expand Down