[BUG] Low-level key module APIs don't send keyspace notifications

[murphyjacob4]

Describe the bug

A module that uses a low-level key API like VM_DeleteKey won't emit keyspace notifications.

Search and client-side caching need keyspace notifications or they will desynchronize from the datastore.

To reproduce

Load a custom module that calls VM_DeleteKey, load ValkeySearch, index a key and then trigger VM_DeleteKey, see that ValkeySearch desyncs.

Expected behavior

Keyspace notifications need to be emitted for every keyspace change, regardless of the reason.

Additional information

I believe this is just an oversight. But if you have a reason to avoid the keyspace notifications, please share.

[enjoy-binbin]

Yes, it seems to be an overlook, it's missing signalModifiedKey / notifyKeyspaceEvent / dirty++.

[zuiderkwast]

Maybe the original idea was that modules need to explicitly call ValkeyModule_NotifyKeyspaceEvent but it's not well documented so I doubt any module does it correctly.

I think we should fix this, but it's a behavior change so maybe we shouldn't backport it. Any modules that explicitly send keyspace events might send duplicated keyspace events if we fix it.

[murphyjacob4]

Hmm, missed ValkeyModule_NotifyKeyspaceEvent and ValkeyModule_SignalModifiedKey. Maybe the original intent here was that modules that use these APIs "know what they are doing(TM)." But previously, we made decisions to do things like call signalModifiedKey implicitly and gave modules the option to opt out (VALKEYMODULE_OPTION_NO_IMPLICIT_SIGNAL_MODIFIED). I will send a PR to do the same here, with an option like VALKEYMODULE_OPTION_NO_IMPLICIT_KEYSPACE_EVENTS.

[zuiderkwast]

If it's urgent for 9.2, we can bring it up on Monday, or maybe it's better to do the change in 10.0?

[murphyjacob4]

Not urgent. I say wait for 10.0 since it would be a possible breaking change.