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

Support publishing of long-lived JFR events to the `EventBus` #2824

Open mp911de opened 2 months ago

mp911de commented 2 months ago

Discussed in https://github.com/redis/lettuce/discussions/2809

Originally posted by **rkeely** April 2, 2024 I would like to know the reason of only publishing `TopologyRefreshEvent` to EventRecorder as implemented [here]( https://github.com/redis/lettuce/blob/8848d151ef902976faa8806e4dafd7d62478e9fd/src/main/java/io/lettuce/core/cluster/RedisClusterClient.java#L888). My app subscribe to all events on event bus but does not receive any `TopologyRefreshEvent` so I assume the current implementation of EventRecorder does not publish to event bus but only JFR, is that correct? Could we consider dual-publishing it to event bus as well? I am happy to create a PR if this idea sounds good. Here are my reasons why we want this: * People normally do not setup recording for JFR in production. We love out of box support without extra effort :D * For Redis cluster with large number of nodes and with large number of clients, the toll on topology refresh will be heavy for both side. As application owner, we would like to monitor the occurrence of TopologyRefreshEvent and make sure there is no un-wanted refresh. I know we can control the refresh periods but unexpected resonation from many clients could happen (e.g. when we scale out our client fleet). Currently we have to use the number connection events to infer the refresh. Being able to easily subscribe to the event could give an concrete answer.