Change default value for KeyspaceEventMessageListener on keyspace event notifications

[jxblum]

Closes #2670

NOTE: the next effect of theses changes is preserving the existing behavior.

While the default value for notifiy-keyspace-events in the abstract class KeyspaceEventMessageListener changed from "EA" to "empty" in this PR, it is an abstract class in the first place, and users must extend the class with a custom implementation in any case. But, I have reluctantly let the default value for the 1 and only Spring Data Redis provided implementation, KeyExpirationEventMessageListener, to "Ex", thereby preserving the existing behavior, i.e. to notify on key expiration events.

This change came about because of #2654, which occurs for 2 reasons.

1. First, the default Redis server configuration setting for notify-keyspace-events is "empty" (or, in other words, disabled). Therefore, on Redis server restarts, any configuration supplied at runtime, such as is the case with a Spring Data Redis application when the KeyExpirationEventMessageListener (or any KeyspaceEventMessageListener) is registered in the Spring ApplicationContext, gets lost! This is even the case for the redis-cli.

2. In addition, any configuration coming from a Spring Data Redis application when a KeyspaceEventMessageListener (such as KeyExpirationEventMessageListener) is declared and registered in the Spring container, only happens once, on initialization. So, when the Redis server restarted, again this configuration is lost!

The user should be specifying Redis server (infrastructure) configuration in redis.conf, which can then also be managed in version control along with the application.

[Lvbuqing]

Thank you for your help, I read your PR carefully: https://github.com/spring-projects/spring-data-redis/pull/2671/commits/98d25f11a601cb57dddd349e07d3231cd357a507

As you said: Why this configuration for the Redis server was even added to Spring Data Redis (i.e. the KeyExpirationEventMessageListener component) to begin with...   mp911de also said: this is both an infrastructure (i.e. "data management policy") and application concern. The framework definitely should not be deciding in this case, no matter how "convenient".

I have a few questions about this PR:

1. EA is deleted here Why is it changed to Ex here?

2. According to the last discussion, in my opinion, we should not configure related properties for Redis, for example, we should not set related values through spring-data-redis，as shown in the figure

keyspaceNotificationsConfigParameter should be used as an expected value, not directly set

[jxblum]

The only reason I set the value to a "Ex" in KeyExpirationEventMessageListener is to preserve existing behavior in the 3.x line. In 4.0, I think this should also be set this value to "empty", even for this listener.

However, I did set the value for notify-keyspace-events in the abstract KeyspaceEventMessageListener to "empty", thereby disabling notifications on all key(space) events.

[christophstrobl]

thanks for picking up this topic. I agree that, though convenient, the Redis config should not be altered by the message listener. However, I'd rather not change defaults in a minor but wait for the next major release. IIRC there's MappingExpirationListener in KeyValueAdapter used in conjunction with TTL, that would interact with the server config, if EnableKeyspaceEvents is set to ON_STARTUP/ON_DEMAND. It's OFF by default in the adapter and EnableRedisRepositories.

[jxblum]

@christophstrobl - Thank you for your feedback.

Correct me if I am wrong, but it seems that 1) the RedisKeyValueAdapter.MessageExpirationListener extends from the SD Redis KeyExpirationEventMessageListener class directly where I kept, though declared a (minimal) value for expiration (i.e. Ex rather than EA; FYR: definitions of E and x) which seems to be all that is required to receive key expiration events.

Perhaps EA is overkill since A is g$lshzxetd (all this as documented in redis.conf), which would only increase the amount of network traffic when pub/sub is enabled when using the SD Redis Repository infrastructure since then Repositories would receive notifications for all events when we are only seemingly interested in expiration events (hence "MappingExpirationListener" as named).

Even the code suggests we filtering for "expiration-only" events, though the logic (and then this) is very cryptic to me. The logic only seems to be checking for the presence of a valid "key" (i.e. keyspace:id) in the message rather than whether the message is really an "expiration" event. That is, I am not sure if other events (e.g. on ADD or DEL) would also contain the key, and if so, what really constitutes an "expiration" message.

[christophstrobl]

Thanks for digging up the documentation, being well aware of the config parameters, my point was about changing defaults in a minor, given there are ways to configure it from outside.

[jxblum]

@christophstrobl - I understood your concern and your point. I was also trying to explain that we are not really changing behavior here since we are still receiving "expiration" events with this configuration.

Ideally, Redis clients are not sending any configuration to the servers, otherwise we are going to (possibly) run into situations exactly like this.

[jxblum]

@christophstrobl - I also stumbled upon and noticed this configuration in SD (Redis) Repositories today (i.e. Ex).