From 8ccb0c98617beb9de1f0441833cd56d899fc8eef Mon Sep 17 00:00:00 2001 From: Victor Polevoy Date: Sun, 3 Dec 2023 15:56:15 +0100 Subject: [PATCH 1/5] Add NEW key space notification flag --- src/raw.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/raw.rs b/src/raw.rs index afca5ceb..866b4c88 100644 --- a/src/raw.rs +++ b/src/raw.rs @@ -134,9 +134,15 @@ bitflags! { const EXPIRED = REDISMODULE_NOTIFY_EXPIRED; const EVICTED = REDISMODULE_NOTIFY_EVICTED; const STREAM = REDISMODULE_NOTIFY_STREAM; + #[cfg(any( + feature = "min-redis-compatibility-version-7-0", + feature = "min-redis-compatibility-version-7-2" + ))] + const NEW = REDISMODULE_NOTIFY_NEW; const MODULE = REDISMODULE_NOTIFY_MODULE; const LOADED = REDISMODULE_NOTIFY_LOADED; const MISSED = REDISMODULE_NOTIFY_KEY_MISS; + /// Does not include the [`Self::MISSED`]. const ALL = REDISMODULE_NOTIFY_ALL; const TRIMMED = REDISMODULE_NOTIFY_TRIMMED; } From 535cc2ed277e61a476521d23277ce8616d235366 Mon Sep 17 00:00:00 2001 From: Victor Polevoy Date: Mon, 4 Dec 2023 10:25:42 +0100 Subject: [PATCH 2/5] Address the review comments --- src/raw.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/raw.rs b/src/raw.rs index 866b4c88..8eefb9c4 100644 --- a/src/raw.rs +++ b/src/raw.rs @@ -134,15 +134,16 @@ bitflags! { const EXPIRED = REDISMODULE_NOTIFY_EXPIRED; const EVICTED = REDISMODULE_NOTIFY_EVICTED; const STREAM = REDISMODULE_NOTIFY_STREAM; - #[cfg(any( - feature = "min-redis-compatibility-version-7-0", - feature = "min-redis-compatibility-version-7-2" - ))] + /// Available only starting from Redis `7.0.1`. const NEW = REDISMODULE_NOTIFY_NEW; const MODULE = REDISMODULE_NOTIFY_MODULE; const LOADED = REDISMODULE_NOTIFY_LOADED; const MISSED = REDISMODULE_NOTIFY_KEY_MISS; - /// Does not include the [`Self::MISSED`]. + /// Does not include the [`Self::MISSED`] and [`Self::NEW`]. + /// + /// Includes [`Self::GENERIC`], [`Self::STRING`], [`Self::LIST`], + /// [`Self::SET`], [`Self::HASH`], [`Self::ZSET`], [`Self::EXPIRED`], + /// [`Self::EVICTED`], [`Self::STREAM`], [`Self::MODULE`]. const ALL = REDISMODULE_NOTIFY_ALL; const TRIMMED = REDISMODULE_NOTIFY_TRIMMED; } From f152f9116f07478b699c2be24f60b789e56b6d57 Mon Sep 17 00:00:00 2001 From: Victor Polevoy Date: Tue, 5 Dec 2023 11:31:10 +0100 Subject: [PATCH 3/5] Don't check the bans for the internal macros crate --- deny.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/deny.toml b/deny.toml index 42fb5141..8151e9bc 100644 --- a/deny.toml +++ b/deny.toml @@ -281,6 +281,10 @@ include-dependencies = true name = "redis-module-macros" allow-globs = ["*"] +[[bans.build.bypass]] +name = "redis-module-macros-internals" +allow-globs = ["*"] + [[bans.build.bypass]] name = "libloading" allow-globs = ["*"] From ab5902fd10f7a793061c79cf2b13c9fb875dde7f Mon Sep 17 00:00:00 2001 From: Victor Polevoy Date: Tue, 5 Dec 2023 11:31:57 +0100 Subject: [PATCH 4/5] Log the unsupported event notification flags when registering the module's subscribers --- examples/events.rs | 10 ++++++++++ src/macros.rs | 10 ++++++++++ src/raw.rs | 18 ++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/examples/events.rs b/examples/events.rs index 48fa7dae..a09c7f2a 100644 --- a/examples/events.rs +++ b/examples/events.rs @@ -5,6 +5,7 @@ use std::ptr::NonNull; use std::sync::atomic::{AtomicI64, Ordering}; static NUM_KEY_MISSES: AtomicI64 = AtomicI64::new(0); +static NUM_KEYS: AtomicI64 = AtomicI64::new(0); fn on_event(ctx: &Context, event_type: NotifyEvent, event: &str, key: &[u8]) { if key == b"num_sets" { @@ -52,6 +53,13 @@ fn num_key_miss(_ctx: &Context, _args: Vec) -> RedisResult { Ok(RedisValue::Integer(NUM_KEY_MISSES.load(Ordering::SeqCst))) } +fn on_new_key(_ctx: &Context, _event_type: NotifyEvent, _event: &str, _key: &[u8]) { + NUM_KEYS.fetch_add(1, Ordering::SeqCst); +} + +fn num_keys(_ctx: &Context, _args: Vec) -> RedisResult { + Ok(RedisValue::Integer(NUM_KEYS.load(Ordering::SeqCst))) +} ////////////////////////////////////////////////////// redis_module! { @@ -62,10 +70,12 @@ redis_module! { commands: [ ["events.send", event_send, "", 0, 0, 0], ["events.num_key_miss", num_key_miss, "", 0, 0, 0], + ["events.num_keys", num_keys, "", 0, 0, 0], ], event_handlers: [ [@STRING: on_event], [@STREAM: on_stream], [@MISSED: on_key_miss], + [@NEW: on_new_key], ], } diff --git a/src/macros.rs b/src/macros.rs index 43b4f21b..0f0faa38 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -68,6 +68,16 @@ macro_rules! redis_event_handler { $crate::raw::Status::Ok as c_int } + let all_available_flags = $crate::raw::get_keyspace_notification_flags_all(); + if !all_available_flags.contains($event_type) { + let not_supported = $event_type.difference(all_available_flags); + log::warn!("The event notification flags set aren't supported: {not_supported:?}"); + + // $crate::Context::new($ctx).log_warning(&format!( + // "The event notification flags set aren't supported: {not_supported:?}" + // )); + } + if unsafe { $crate::raw::RedisModule_SubscribeToKeyspaceEvents.unwrap()( $ctx, diff --git a/src/raw.rs b/src/raw.rs index 8eefb9c4..7d779a01 100644 --- a/src/raw.rs +++ b/src/raw.rs @@ -912,6 +912,24 @@ pub fn get_keyspace_events() -> NotifyEvent { } } +/// Returns all the available notification flags for key-space +/// notifications. +/// +/// # Safety +/// +/// This function is safe to use as it doesn't perform any work with +/// the [RedisModuleCtx] pointer except for passing it to the redis server. +/// +/// # Panics +/// +/// Panics when the [RedisModule_GetKeyspaceNotificationFlagsAll] is +/// unavailable. +pub fn get_keyspace_notification_flags_all() -> NotifyEvent { + unsafe { + NotifyEvent::from_bits_truncate(RedisModule_GetKeyspaceNotificationFlagsAll.unwrap()()) + } +} + #[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct Version { pub major: i32, From 023af7f9690e03cf6a37a1947bdb35a35d44d2b4 Mon Sep 17 00:00:00 2001 From: Victor Polevoy Date: Tue, 5 Dec 2023 14:14:26 +0100 Subject: [PATCH 5/5] Allow subscribing only with the supported flags. --- src/macros.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 0f0faa38..0faf213f 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -68,20 +68,19 @@ macro_rules! redis_event_handler { $crate::raw::Status::Ok as c_int } - let all_available_flags = $crate::raw::get_keyspace_notification_flags_all(); - if !all_available_flags.contains($event_type) { - let not_supported = $event_type.difference(all_available_flags); - log::warn!("The event notification flags set aren't supported: {not_supported:?}"); - - // $crate::Context::new($ctx).log_warning(&format!( - // "The event notification flags set aren't supported: {not_supported:?}" - // )); + let all_available_notification_flags = $crate::raw::get_keyspace_notification_flags_all(); + let available_wanted_notification_flags = $event_type.intersection(all_available_notification_flags); + if !all_available_notification_flags.contains($event_type) { + let not_supported = $event_type.difference(all_available_notification_flags); + $crate::Context::new($ctx).log_notice(&format!( + "These event notification flags set aren't supported: {not_supported:?}. These flags will be used: {available_wanted_notification_flags:?}" + )); } - if unsafe { + if !available_wanted_notification_flags.is_empty() && unsafe { $crate::raw::RedisModule_SubscribeToKeyspaceEvents.unwrap()( $ctx, - $event_type.bits(), + available_wanted_notification_flags.bits(), Some(__handle_event), ) } == $crate::raw::Status::Err as c_int