See
#1819 (comment)

Verified the fix by:
1. Build: `make SERVER_CFLAGS='-Werror -DLOG_REQ_RES' -j10`
2. Running module api tests: `CFLAGS='-Werror' ./runtest-moduleapi
--log-req-res --no-latency --dont-clean --force-resp3 --dont-pre-clean
--verbose --dump-logs`

---------

Signed-off-by: yairgott <yairgott@gmail.com>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>

…lkey-io#1819)

This change enhances user experience and consistency by allowing a
module to block a client on keyspace event notifications. Consistency is
improved by allowing that reads after writes on the same connection
yield expected results. For example, in ValkeySearch, mutations
processed earlier on the same connection will be available for search.

The implementation extends `VM_BlockClient` to support blocking clients
on keyspace event notifications. Internal clients, LUA clients, clients
issueing multi exec and those with the `deny_blocking` flag set are not
blocked. Once blocked, a client’s reply is withheld until it is
explicitly unblocked.

---------

Signed-off-by: yairgott <yairgott@gmail.com>
Signed-off-by: hwware <wen.hui.ware@gmail.com>

See
valkey-io#1819 (comment)

Verified the fix by:
1. Build: `make SERVER_CFLAGS='-Werror -DLOG_REQ_RES' -j10`
2. Running module api tests: `CFLAGS='-Werror' ./runtest-moduleapi
--log-req-res --no-latency --dont-clean --force-resp3 --dont-pre-clean
--verbose --dump-logs`

---------

Signed-off-by: yairgott <yairgott@gmail.com>
Signed-off-by: hwware <wen.hui.ware@gmail.com>

See
valkey-io#1819 (comment)

Verified the fix by:
1. Build: `make SERVER_CFLAGS='-Werror -DLOG_REQ_RES' -j10`
2. Running module api tests: `CFLAGS='-Werror' ./runtest-moduleapi
--log-req-res --no-latency --dont-clean --force-resp3 --dont-pre-clean
--verbose --dump-logs`

---------

Signed-off-by: yairgott <yairgott@gmail.com>

This flag.deny_blocking check causes some code to becomde dead code.
Some of the code below checks islua or ismulti, but they are marked
with flag.deny_blocking and will return early.

In addition, cleanup some documents, some of them are inaccurate,
and restore the code of blocking_auth_cb in tests/modules/auth.c,
this reply should be returned from core. The reply used to from the
core and was changed to from the module in valkey-io#1819, and now it is from
the core again.

Cleanup some dead code around valkey-io#1819.

Signed-off-by: Binbin <binloveplay1314@qq.com>

When we add f8a5a4f, a module auth
can add reply in its callback, or not add reply and just return an
error robj in its callback, and we have to figure a way to determine
whether it has added the reply.

Before we are using `clientHasPendingReplies` and it had a issue,
the auth might be used inside a transaction or pipeline which already
has some pending replies in the client reply list. It causes us to
not add the error reply to auth.

After valkey-io#1819, now we have a buffered_reply client flag, it indicates
the reply for the current command was buffered, either in client::reply
or in client::buf. We can use this flag to check.

Fixes valkey-io#2106.

Signed-off-by: Binbin <binloveplay1314@qq.com>

When we add f8a5a4f, a module auth
can add reply in its callback, or not add reply and just return an
error robj in its callback, and we have to figure a way to determine
whether it has added the reply.

Before we are using `clientHasPendingReplies` and it had a issue,
the auth might be used inside a transaction or pipeline which already
has some pending replies in the client reply list. It causes us to
not add the error reply to auth.

After #1819, now we have a buffered_reply client flag, it indicates
the reply for the current command was buffered, either in client::reply
or in client::buf. We can use this flag to check.

Fixes #2106.

---------

Signed-off-by: Binbin <binloveplay1314@qq.com>

…y-io#2287)

When we add f8a5a4f, a module auth
can add reply in its callback, or not add reply and just return an
error robj in its callback, and we have to figure a way to determine
whether it has added the reply.

Before we are using `clientHasPendingReplies` and it had a issue,
the auth might be used inside a transaction or pipeline which already
has some pending replies in the client reply list. It causes us to
not add the error reply to auth.

After valkey-io#1819, now we have a buffered_reply client flag, it indicates
the reply for the current command was buffered, either in client::reply
or in client::buf. We can use this flag to check.

Fixes valkey-io#2106.

---------

Signed-off-by: Binbin <binloveplay1314@qq.com>

…#2215)

This flag.deny_blocking check causes some code to becomde dead code.
Some of the code below checks islua or ismulti, but they are marked
with flag.deny_blocking and will return early.

In addition, cleanup some documents, some of them are inaccurate,
and restore the code of blocking_auth_cb in tests/modules/auth.c,
this reply should be returned from core. The reply used to from the
core and was changed to from the module in #1819, and now it is from
the core again.

Cleanup some dead code around #1819.

Signed-off-by: Binbin <binloveplay1314@qq.com>

…valkey-io#2215)

This flag.deny_blocking check causes some code to becomde dead code.
Some of the code below checks islua or ismulti, but they are marked
with flag.deny_blocking and will return early.

In addition, cleanup some documents, some of them are inaccurate,
and restore the code of blocking_auth_cb in tests/modules/auth.c,
this reply should be returned from core. The reply used to from the
core and was changed to from the module in valkey-io#1819, and now it is from
the core again.

Cleanup some dead code around valkey-io#1819.

Signed-off-by: Binbin <binloveplay1314@qq.com>

…#2215)

This flag.deny_blocking check causes some code to becomde dead code.
Some of the code below checks islua or ismulti, but they are marked
with flag.deny_blocking and will return early.

In addition, cleanup some documents, some of them are inaccurate,
and restore the code of blocking_auth_cb in tests/modules/auth.c,
this reply should be returned from core. The reply used to from the
core and was changed to from the module in #1819, and now it is from
the core again.

Cleanup some dead code around #1819.

Signed-off-by: Binbin <binloveplay1314@qq.com>

…valkey-io#2215)

This flag.deny_blocking check causes some code to becomde dead code.
Some of the code below checks islua or ismulti, but they are marked
with flag.deny_blocking and will return early.

In addition, cleanup some documents, some of them are inaccurate,
and restore the code of blocking_auth_cb in tests/modules/auth.c,
this reply should be returned from core. The reply used to from the
core and was changed to from the module in valkey-io#1819, and now it is from
the core again.

Cleanup some dead code around valkey-io#1819.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Harkrishn Patro <harkrisp@amazon.com>

…cations (#3743)

When a module subscribes to NOTIFY_HASH keyspace events and blocks the
client in the notification callback, hexpireGenericCommand,
hgetdelCommand,
and hpersistCommand would trigger debugServerAssert in
notifyKeyspaceEvent()
because addReplyArrayLen() sets buffered_reply=1 before the notification
fires. This debug assert was added in #1819.

Affected commands: HEXPIRE, HPEXPIRE, HEXPIREAT, HPEXPIREAT, HGETDEL,
HPERSIST.

---------

Signed-off-by: Binbin <binloveplay1314@qq.com>

…cations (#3743)

When a module subscribes to NOTIFY_HASH keyspace events and blocks the
client in the notification callback, hexpireGenericCommand,
hgetdelCommand,
and hpersistCommand would trigger debugServerAssert in
notifyKeyspaceEvent()
because addReplyArrayLen() sets buffered_reply=1 before the notification
fires. This debug assert was added in #1819.

Affected commands: HEXPIRE, HPEXPIRE, HEXPIREAT, HPEXPIREAT, HGETDEL,
HPERSIST.

---------

Signed-off-by: Binbin <binloveplay1314@qq.com>

…cations (#3743)

When a module subscribes to NOTIFY_HASH keyspace events and blocks the
client in the notification callback, hexpireGenericCommand,
hgetdelCommand,
and hpersistCommand would trigger debugServerAssert in
notifyKeyspaceEvent()
because addReplyArrayLen() sets buffered_reply=1 before the notification
fires. This debug assert was added in #1819.

Affected commands: HEXPIRE, HPEXPIRE, HEXPIREAT, HPEXPIREAT, HGETDEL,
HPERSIST.

---------

Signed-off-by: Binbin <binloveplay1314@qq.com>

…cations (#3743)

When a module subscribes to NOTIFY_HASH keyspace events and blocks the
client in the notification callback, hexpireGenericCommand,
hgetdelCommand,
and hpersistCommand would trigger debugServerAssert in
notifyKeyspaceEvent()
because addReplyArrayLen() sets buffered_reply=1 before the notification
fires. This debug assert was added in #1819.

Affected commands: HEXPIRE, HPEXPIRE, HEXPIREAT, HPEXPIREAT, HGETDEL,
HPERSIST.

---------

Signed-off-by: Binbin <binloveplay1314@qq.com>

…cations (#3743)

When a module subscribes to NOTIFY_HASH keyspace events and blocks the
client in the notification callback, hexpireGenericCommand,
hgetdelCommand,
and hpersistCommand would trigger debugServerAssert in
notifyKeyspaceEvent()
because addReplyArrayLen() sets buffered_reply=1 before the notification
fires. This debug assert was added in #1819.

Affected commands: HEXPIRE, HPEXPIRE, HEXPIREAT, HPEXPIREAT, HGETDEL,
HPERSIST.

---------

Signed-off-by: Binbin <binloveplay1314@qq.com>

…cations (#3743)

When a module subscribes to NOTIFY_HASH keyspace events and blocks the
client in the notification callback, hexpireGenericCommand,
hgetdelCommand,
and hpersistCommand would trigger debugServerAssert in
notifyKeyspaceEvent()
because addReplyArrayLen() sets buffered_reply=1 before the notification
fires. This debug assert was added in #1819.

Affected commands: HEXPIRE, HPEXPIRE, HEXPIREAT, HPEXPIREAT, HGETDEL,
HPERSIST.

---------

Signed-off-by: Binbin <binloveplay1314@qq.com>

moduleNotifyKeyspaceEvent() runs subscriber callbacks on the executing
client and calls selectDb(dbid) on it, but never restores the DB. When
dbid differs from the client's current DB (e.g. MOVE/COPY notify on the
destination DB), the client is left on the wrong DB, breaking subsequent
commands.

This was introduced by valkey-io#1819, which started reusing the executing client
instead of a temporary one. Save the executing client's DB before the
subscriber loop and restore it afterwards. Covers both MOVE and COPY.

Signed-off-by: Binbin <binloveplay1314@qq.com>

…#3381)

Under reply-blocking, module keyspace notifications were delivered only
via the deferred post-commit task, which is registered only when the
client's notify-keyspace-events matches the event. With the default
empty config, or for background jobs (expiry/eviction) that bypass
certification, module subscribers were dropped entirely -- yet modules
filter by their own event mask and should see every event. Notify
modules inline (as upstream does) and defer only the client pub/sub;
certification no longer re-notifies modules.

Inline notification also lets a module block the client from its
callback (valkey-io#1819) under reply-blocking. To keep that durable, re-apply
the reply-blocking boundary when the parked reply is committed on
unblock, unless the write is already acked. The boundary is snapshotted
at commit time, not reused from command start, to avoid a use-after-free
on a reply buffer node freed during the block.

Add tests/unit/moduleapi/block_keyspace_notification_durability.tcl.

Signed-off-by: Chris Li <1070743423@qq.com>

…#3381)

Under reply-blocking, module keyspace notifications were delivered only
via the deferred post-commit task, which is registered only when the
client's notify-keyspace-events matches the event. With the default
empty config, or for background jobs (expiry/eviction) that bypass
certification, module subscribers were dropped entirely -- yet modules
filter by their own event mask and should see every event. Notify
modules inline (as upstream does) and defer only the client pub/sub;
certification no longer re-notifies modules.

Inline notification also lets a module block the client from its
callback (valkey-io#1819) under reply-blocking. To keep that durable, re-apply
the reply-blocking boundary when the parked reply is committed on
unblock, unless the write is already acked. The boundary is snapshotted
at commit time, not reused from command start, to avoid a use-after-free
on a reply buffer node freed during the block.

Add tests/unit/moduleapi/block_keyspace_notification_durability.tcl.

Signed-off-by: Chris Li <1070743423@qq.com>