Skip to content

Add DENYOOM flag to SUBSCRIBE, PSUBSCRIBE, SSUBSCRIBE, WATCH #4230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
enjoy-binbin wants to merge 1 commit into
base: unstable
Choose a base branch
from
Open

Add DENYOOM flag to SUBSCRIBE, PSUBSCRIBE, SSUBSCRIBE, WATCH #4230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/commands.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11995,13 +11995,13 @@ struct COMMAND_STRUCT serverCommandTable[] = {
{MAKE_CMD("rpush","Appends one or more elements to a list. Creates the key if it doesn't exist.","O(1) for each element added, so O(N) to add N elements when the command is called with multiple arguments.","1.0.0",CMD_DOC_NONE,NULL,NULL,"list",COMMAND_GROUP_LIST,RPUSH_History,1,RPUSH_Tips,0,rpushCommand,-3,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_FAST|ACL_CATEGORY_LIST|ACL_CATEGORY_WRITE,NULL,RPUSH_Keyspecs,1,NULL,2),.args=RPUSH_Args},
{MAKE_CMD("rpushx","Appends one or more elements to a list only when the list exists.","O(1) for each element added, so O(N) to add N elements when the command is called with multiple arguments.","2.2.0",CMD_DOC_NONE,NULL,NULL,"list",COMMAND_GROUP_LIST,RPUSHX_History,1,RPUSHX_Tips,0,rpushxCommand,-3,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_FAST|ACL_CATEGORY_LIST|ACL_CATEGORY_WRITE,NULL,RPUSHX_Keyspecs,1,NULL,2),.args=RPUSHX_Args},
/* pubsub */
{MAKE_CMD("psubscribe","Listens for messages published to channels that match one or more patterns.","O(N) where N is the number of patterns to subscribe to.","2.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PSUBSCRIBE_History,0,PSUBSCRIBE_Tips,0,psubscribeCommand,-2,CMD_PUBSUB|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_PUBSUB|ACL_CATEGORY_SLOW,NULL,PSUBSCRIBE_Keyspecs,0,NULL,1),.args=PSUBSCRIBE_Args},
{MAKE_CMD("psubscribe","Listens for messages published to channels that match one or more patterns.","O(N) where N is the number of patterns to subscribe to.","2.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PSUBSCRIBE_History,0,PSUBSCRIBE_Tips,0,psubscribeCommand,-2,CMD_PUBSUB|CMD_DENYOOM|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_PUBSUB|ACL_CATEGORY_SLOW,NULL,PSUBSCRIBE_Keyspecs,0,NULL,1),.args=PSUBSCRIBE_Args},
{MAKE_CMD("publish","Posts a message to a channel.","O(N+M) where N is the number of clients subscribed to the receiving channel and M is the total number of subscribed patterns (by any client).","2.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PUBLISH_History,0,PUBLISH_Tips,0,publishCommand,3,CMD_PUBSUB|CMD_LOADING|CMD_STALE|CMD_FAST|CMD_MAY_REPLICATE|CMD_SENTINEL,ACL_CATEGORY_FAST|ACL_CATEGORY_PUBSUB,NULL,PUBLISH_Keyspecs,0,NULL,2),.args=PUBLISH_Args},
{MAKE_CMD("pubsub","A container for Pub/Sub commands.","Depends on subcommand.","2.8.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PUBSUB_History,0,PUBSUB_Tips,0,NULL,-2,0,ACL_CATEGORY_SLOW,NULL,PUBSUB_Keyspecs,0,NULL,0),.subcommands=PUBSUB_Subcommands},
{MAKE_CMD("punsubscribe","Stops listening to messages published to channels that match one or more patterns.","O(N) where N is the number of patterns to unsubscribe.","2.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PUNSUBSCRIBE_History,0,PUNSUBSCRIBE_Tips,0,punsubscribeCommand,-1,CMD_PUBSUB|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_PUBSUB|ACL_CATEGORY_SLOW,NULL,PUNSUBSCRIBE_Keyspecs,0,NULL,1),.args=PUNSUBSCRIBE_Args},
{MAKE_CMD("spublish","Posts a message to a shard channel.","O(N) where N is the number of clients subscribed to the receiving shard channel.","7.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,SPUBLISH_History,0,SPUBLISH_Tips,0,spublishCommand,3,CMD_PUBSUB|CMD_LOADING|CMD_STALE|CMD_FAST|CMD_MAY_REPLICATE,ACL_CATEGORY_FAST|ACL_CATEGORY_PUBSUB,NULL,SPUBLISH_Keyspecs,1,NULL,2),.args=SPUBLISH_Args},
{MAKE_CMD("ssubscribe","Listens for messages published to shard channels.","O(N) where N is the number of shard channels to subscribe to.","7.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,SSUBSCRIBE_History,0,SSUBSCRIBE_Tips,0,ssubscribeCommand,-2,CMD_PUBSUB|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE,ACL_CATEGORY_PUBSUB|ACL_CATEGORY_SLOW,NULL,SSUBSCRIBE_Keyspecs,1,NULL,1),.args=SSUBSCRIBE_Args},
{MAKE_CMD("subscribe","Listens for messages published to channels.","O(N) where N is the number of channels to subscribe to.","2.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,SUBSCRIBE_History,0,SUBSCRIBE_Tips,0,subscribeCommand,-2,CMD_PUBSUB|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_PUBSUB|ACL_CATEGORY_SLOW,NULL,SUBSCRIBE_Keyspecs,0,NULL,1),.args=SUBSCRIBE_Args},
{MAKE_CMD("ssubscribe","Listens for messages published to shard channels.","O(N) where N is the number of shard channels to subscribe to.","7.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,SSUBSCRIBE_History,0,SSUBSCRIBE_Tips,0,ssubscribeCommand,-2,CMD_PUBSUB|CMD_DENYOOM|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE,ACL_CATEGORY_PUBSUB|ACL_CATEGORY_SLOW,NULL,SSUBSCRIBE_Keyspecs,1,NULL,1),.args=SSUBSCRIBE_Args},
{MAKE_CMD("subscribe","Listens for messages published to channels.","O(N) where N is the number of channels to subscribe to.","2.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,SUBSCRIBE_History,0,SUBSCRIBE_Tips,0,subscribeCommand,-2,CMD_PUBSUB|CMD_DENYOOM|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_PUBSUB|ACL_CATEGORY_SLOW,NULL,SUBSCRIBE_Keyspecs,0,NULL,1),.args=SUBSCRIBE_Args},
{MAKE_CMD("sunsubscribe","Stops listening to messages posted to shard channels.","O(N) where N is the number of shard channels to unsubscribe.","7.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,SUNSUBSCRIBE_History,0,SUNSUBSCRIBE_Tips,0,sunsubscribeCommand,-1,CMD_PUBSUB|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE,ACL_CATEGORY_PUBSUB|ACL_CATEGORY_SLOW,NULL,SUNSUBSCRIBE_Keyspecs,1,NULL,1),.args=SUNSUBSCRIBE_Args},
{MAKE_CMD("unsubscribe","Stops listening to messages posted to channels.","O(N) where N is the number of channels to unsubscribe.","2.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,UNSUBSCRIBE_History,0,UNSUBSCRIBE_Tips,0,unsubscribeCommand,-1,CMD_PUBSUB|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_PUBSUB|ACL_CATEGORY_SLOW,NULL,UNSUBSCRIBE_Keyspecs,0,NULL,1),.args=UNSUBSCRIBE_Args},
/* scripting */
Expand Down Expand Up @@ -12146,6 +12146,6 @@ struct COMMAND_STRUCT serverCommandTable[] = {
{MAKE_CMD("exec","Executes all commands in a transaction.","Depends on commands in the transaction","1.2.0",CMD_DOC_NONE,NULL,NULL,"transactions",COMMAND_GROUP_TRANSACTIONS,EXEC_History,0,EXEC_Tips,0,execCommand,1,CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_SKIP_COMMANDLOG,ACL_CATEGORY_SLOW|ACL_CATEGORY_TRANSACTION,NULL,EXEC_Keyspecs,0,NULL,0)},
{MAKE_CMD("multi","Starts a transaction.","O(1)","1.2.0",CMD_DOC_NONE,NULL,NULL,"transactions",COMMAND_GROUP_TRANSACTIONS,MULTI_History,0,MULTI_Tips,0,multiCommand,1,CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_FAST|CMD_NO_MULTI|CMD_ALLOW_BUSY,ACL_CATEGORY_FAST|ACL_CATEGORY_TRANSACTION,NULL,MULTI_Keyspecs,0,NULL,0)},
{MAKE_CMD("unwatch","Forgets about watched keys of a transaction.","O(1)","2.2.0",CMD_DOC_NONE,NULL,NULL,"transactions",COMMAND_GROUP_TRANSACTIONS,UNWATCH_History,0,UNWATCH_Tips,0,unwatchCommand,1,CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_FAST|CMD_ALLOW_BUSY,ACL_CATEGORY_FAST|ACL_CATEGORY_TRANSACTION,NULL,UNWATCH_Keyspecs,0,NULL,0)},
{MAKE_CMD("watch","Monitors changes to keys to determine the execution of a transaction.","O(1) for every key.","2.2.0",CMD_DOC_NONE,NULL,NULL,"transactions",COMMAND_GROUP_TRANSACTIONS,WATCH_History,0,WATCH_Tips,0,watchCommand,-2,CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_FAST|CMD_NO_MULTI|CMD_ALLOW_BUSY,ACL_CATEGORY_FAST|ACL_CATEGORY_TRANSACTION,NULL,WATCH_Keyspecs,1,NULL,1),.args=WATCH_Args},
{MAKE_CMD("watch","Monitors changes to keys to determine the execution of a transaction.","O(1) for every key.","2.2.0",CMD_DOC_NONE,NULL,NULL,"transactions",COMMAND_GROUP_TRANSACTIONS,WATCH_History,0,WATCH_Tips,0,watchCommand,-2,CMD_DENYOOM|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_FAST|CMD_NO_MULTI|CMD_ALLOW_BUSY,ACL_CATEGORY_FAST|ACL_CATEGORY_TRANSACTION,NULL,WATCH_Keyspecs,1,NULL,1),.args=WATCH_Args},
{0}
};
1 change: 1 addition & 0 deletions src/commands/psubscribe.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"function": "psubscribeCommand",
"command_flags": [
"PUBSUB",
"DENYOOM",
"NOSCRIPT",
"LOADING",
"STALE",
Expand Down
1 change: 1 addition & 0 deletions src/commands/ssubscribe.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"function": "ssubscribeCommand",
"command_flags": [
"PUBSUB",
"DENYOOM",
"NOSCRIPT",
"LOADING",
"STALE"
Expand Down
1 change: 1 addition & 0 deletions src/commands/subscribe.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"function": "subscribeCommand",
"command_flags": [
"PUBSUB",
"DENYOOM",

@valkey-review-bot valkey-review-bot Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUBSCRIBE is also the command Sentinel uses for its internal __sentinel__:hello pubsub link (src/sentinel.c:2411-2413). With DENYOOM here, an instance that is already over maxmemory rejects that subscription in processCommand() (src/server.c:4550-4570), so Sentinel never actually joins the hello channel. The callback on that link only handles pubsub message frames and ignores the error reply (src/sentinel.c:2879-2900), so the connection sits unsubscribed until the inactivity timeout closes and retries it (src/sentinel.c:4472-4474).

This needs an exemption for Sentinel’s internal hello subscription instead of making SUBSCRIBE deny-oom unconditionally.

"NOSCRIPT",
"LOADING",
"STALE",
Expand Down
1 change: 1 addition & 0 deletions src/commands/watch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"arity": -2,
"function": "watchCommand",
"command_flags": [
"DENYOOM",
"NOSCRIPT",
"LOADING",
"STALE",
Expand Down
83 changes: 83 additions & 0 deletions tests/unit/maxmemory.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,3 +609,86 @@ start_server {tags {"maxmemory" "external:skip"}} {
assert_equal [r dbsize] {0}
}
}

start_server {tags {"maxmemory" "external:skip"}} {
test {SUBSCRIBE, PSUBSCRIBE, SSUBSCRIBE and WATCH should be denied with OOM with noeviction} {
r flushall sync
r config set maxmemory 100MB
r config set maxmemory-policy noeviction

r set smallkey hello
r set bigkey [string repeat x 1000000]

r config set maxmemory 1
assert_error {OOM command not allowed*} {r subscribe test_channel}
assert_error {OOM command not allowed*} {r psubscribe test_pattern*}
assert_error {OOM command not allowed*} {r ssubscribe test_shardchannel}
assert_error {OOM command not allowed*} {r watch test_key}

# Keys are never evicted under noeviction, so they are all still there.
assert_equal [r dbsize] 2

r config set maxmemory 0
r config set maxmemory-policy noeviction
} {OK} {needs:config-maxmemory}

test {SUBSCRIBE, PSUBSCRIBE, SSUBSCRIBE and WATCH should be denied with OOM even when eviction cannot free enough memory} {
r flushall sync
r config set maxmemory 100MB
r config set maxmemory-policy allkeys-lru

r set smallkey hello
r set bigkey [string repeat x 1000000]

# maxmemory is far too low to ever be reached by eviction, so the
# denyoom commands must still be rejected even with an eviction policy.
r config set maxmemory 1
assert_error {OOM command not allowed*} {r subscribe test_channel}
assert_error {OOM command not allowed*} {r psubscribe test_pattern*}
assert_error {OOM command not allowed*} {r ssubscribe test_shardchannel}
assert_error {OOM command not allowed*} {r watch test_key}

# Eviction was attempted on every command above and emptied the dataset.
assert_equal [r dbsize] 0

r config set maxmemory 0
r config set maxmemory-policy noeviction
} {OK} {needs:config-maxmemory}

test {SUBSCRIBE, PSUBSCRIBE, SSUBSCRIBE and WATCH are allowed when below maxmemory} {
r flushall sync
r config set maxmemory 100MB
r config set maxmemory-policy noeviction

set rd [valkey_deferring_client]
assert_equal {1} [subscribe $rd test_channel]
assert_equal {2} [psubscribe $rd test_pattern*]
assert_equal {1} [ssubscribe $rd test_shardchannel]
$rd close

assert_equal {OK} [r watch test_key]
assert_equal {OK} [r unwatch]

r config set maxmemory 0
} {OK} {needs:config-maxmemory}

test {UNSUBSCRIBE, PUNSUBSCRIBE, SUNSUBSCRIBE, UNWATCH, PUBLISH and SPUBLISH are not denied with OOM} {
r flushall sync
r config set maxmemory 100MB
r config set maxmemory-policy noeviction

r set bigkey [string repeat x 1000000]

# These commands do not carry the DENYOOM flag (they don't grow memory),
# so they must keep working even when the server is out of memory.
r config set maxmemory 1
assert_equal {OK} [r unwatch]
assert_equal {unsubscribe {} 0} [r unsubscribe]
assert_equal {punsubscribe {} 0} [r punsubscribe]
assert_equal {sunsubscribe {} 0} [r sunsubscribe]
assert_equal {0} [r publish test_channel hello]
assert_equal {0} [r spublish test_shardchannel hello]

r config set maxmemory 0
} {OK} {needs:config-maxmemory}
}
Loading