valkey icon indicating copy to clipboard operation
valkey copied to clipboard

Implement MODULE RELOAD command

Open dmitrypol opened this issue 1 year ago • 3 comments

The problem/use-case that the feature addresses

Current you can do MODULE LOAD and MODULE UNLOAD. LOAD takes path and UNLOAD takes name. But sometimes you want to reload the module.

Description of the feature

New RELOAD command. If module is loaded it would unload and load. If not loaded it would just load. It would need name and path plus any args to configure module.

Alternatives you've considered

Yes, you can make due with the current functionality but this would be a nice improvement.

Additional information

I am willing to do the work and submit the PR if people agree.

dmitrypol avatar Apr 21 '24 16:04 dmitrypol

Would this be purely quality of life, or would this be like an "atomic reload" where the clients would not see disruption in module commands?

We might get atomic reload for free if we just do the UNLOAD and LOAD synchronously on the main thread. I can see this being useful for operators to upgrade modules without needing to failover, supposing module LOAD and UNLOAD are quick.

murphyjacob4 avatar Apr 21 '24 17:04 murphyjacob4

We might get atomic reload for free if we just do the UNLOAD and LOAD synchronously on the main thread. I can see this being useful for operators to upgrade modules without needing to failover, supposing module LOAD and UNLOAD are quick.

You can do that today with multi + load + unload. Not sure we need a command for that. It's worth mentioning that UNLOAD was basically a QoL feature at the time. I don't know anyone that actually unloads in production. Adding RELOAD does allow one use case, which is allows reloading modules that can't unload (for example, modules that register a type can't be unloaded).

You could potentially also allow state handover. This would allow more efficient upgrades. There was other discussions about setting module private data, which could then be handed over.

madolson avatar Apr 21 '24 18:04 madolson

RELOAD is an interesting idea, but I think it is more difficult to implement. In our production process, a very troublesome point is how to roll the instances of the module.

Due to many restrictions of unload, such as type, usedby, etc., in the production environment, the instances must be restarted to realize the rolling of the module, which will trigger the reload operation of the rdb.

At this stage, a better way is to implement the division of the module according to the function, such as the API module and the Data module. At this time, we can achieve the RELOAD capability by cutting the flow + UNLOAD + LOAD operation.

So, maybe what we need more is the best practice of how to design a Module, rather than a RELOAD method?

artikell avatar Apr 22 '24 03:04 artikell