Skip to content

Conversation

kfindeisen
Copy link
Member

This PR refactors the Redis connection management to something with lower algorithmic complexity, then adds a check to both Knative and Keda that rejects messages sent too long ago.

@kfindeisen kfindeisen force-pushed the tickets/DM-49427 branch 2 times, most recently from fd10743 to fa335ff Compare April 2, 2025 17:57
@kfindeisen kfindeisen requested a review from dspeck1 April 2, 2025 21:02
Copy link
Collaborator

@dspeck1 dspeck1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice work!

The RedisStreamSession session takes care of making sure a connection
is open, and closes on stream errors, and these two actions are
completely decoupled from each other. This should be easier to
understand and modify than the previous branch-based logic.
This change removes the last of the client micromanagement from the
main Keda execution loop. Behavior is unchanged.
This change lets FannedOutVisit be used as a universal message
representation in all other code. It also has the benefit that if the
message is invalid, the stream is not closed.

I considered moving the FAnnedOutVisit conversion into
RedisStreamSession, but this way keeps the class "clean" with only
Redis-specific elements.
This prevents the service from being overwhelmed in cases where the
original fan-out message was delayed by server problems, as well as
cases where processing can't keep up and develops a backlog.
This prevents the service from being overwhelmed without depending on
the existing safeguards in the Fan-Out service or Knative itself.
@kfindeisen kfindeisen merged commit 93d3ad4 into main Apr 3, 2025
10 checks passed
@kfindeisen kfindeisen deleted the tickets/DM-49427 branch April 3, 2025 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants