redis / lettuce

Advanced Java Redis client for thread-safe sync, async, and reactive usage. Supports Cluster, Sentinel, Pipelining, and codecs.
https://lettuce.io
MIT License
5.3k stars 949 forks source link

Initial commit for dual publishing TopologyRefreshEvent to EventBus #2819

Closed rkeely closed 1 month ago

rkeely commented 2 months ago

Publish TopologyRefreshEvent to event bus #2809

Update: 2024-04-07

Only include interface change to get a sense of direction before larger change.

Some reasoning:

  1. Creating a new interface called DurationalEvent to capture the traits of event thats not ephemeral or instantaneous. I didn't reuse the Event interface because most of events that implements it are instantaneous and I don't want them to carry traits that could indicate anything that's related to duration.
  2. Adding new methods in EventRecorder to explicitly handle dual publishing (recording with recorder and publishing with event bus or only recording). This seems to be the most straight forward and I am also open to creating a new wrapper class for dual publishing only.
  3. Reusing TopologyRefreshEvent as I suspect there are many people subscribing to it from EventBus already. I am also open to create new events class dedicated for the start and end for topology refresh operation.

Make sure that:

mp911de commented 2 months ago

Extending individual events requires duplicating a lot of the functionality within the individual events and it makes these mutable.

I was looking more for an approach where we extend the recording/publishing infrastructure. In particular:

rkeely commented 1 month ago

Sorry that I deleted my PR branch while doing some rebase. I have created a new PR https://github.com/redis/lettuce/pull/2859. Let's continue the discussion in the new one