Skip to content
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.

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

Allow loading RDB with module AUX data even if module is missing. #11056

Commits on Jul 28, 2022

  1. Allow loading RDB with module AUX data even if module is missing.

    An issue that we encounter recently is a user that loaded a module that saves AUX data. The user ended up not using the module, but the RDB that was created did contain the module AUX data. The user ended up not been able to load his data without the module (even though he is not using the module).
    
    Notice that even if the module saves nothing on the module AUX save callback, the module AUX load will still be called when loading the RDB and will failed if the module is missing. So the problem can not be solved on the module level and requires changes in Redis.
    
    ## The Solution
    
    We believe the AUX data is used to save module metadata and it is OK to simply skip it if its not there. This is what the PR does, if module is not loaded we just skip its AUX data. To conclude:
    
    * Module not loaded -> skip module aux data
    * Module loaded -> call the module load callback
    
    ## Other Solutions to Consider
    
    The other solutions attempt to recongnize the case where the module wrote nothing on the AUX save callback and allow loading the RDB (even if the module is missing) in this case only.
    
    1. Skip module AUX data only if **no data was actually written to the RDB by the module AUX save** callback and the **module is not loaded**. If the module is loaded we can not skip calling the load callback because otherwise it can be consdidered a breaking change. So we have to pass the responsibility of **not** reading any data to the module, but the module can not know that he has nothing to read without reading something :). So we need to give the module an ability to check that there is nothing to read. We can do it with another module API, `RM_LoadHasData`, that will tell the module if something was written to the AUX field. To conclude:
        * Aux field is empty ->
          * Module not loaded -> skip
          * Module loaded -> call the module load callback, module will need to use `RM_LoadHasData` to know if there is a data to read.
        * Aux field not empty ->
          * Module not loaded -> fail
          * Module loaded -> call the module load callback.
    
    2. Break the existing API and decide that if nothing was written on the save callback then we will not call the load callback (even if the module is exists). This can be considered a breaking change but its easier solution then (1). To conclude:
        * Aux field is empty ->
          * Module not loaded -> skip
          * Module loaded -> skip
        * Aux field not empty ->
          * Module not loaded -> fail
          * Module loaded -> call the module load callback.
    
    3. Same as 2 but with some datatype flag to avoid breaking change. To conclude:
        * Aux field is empty ->
          * Module not loaded -> skip
          * Module loaded -> check datatype flag to decide whether or not to call the module load callback
        * Aux field not empty ->
          * Module not loaded -> fail
          * Module loaded -> call the module load callback.
    
    We do believe the solution in the PR is good enough but we want to hear other opinions.
    MeirShpilraien committed Jul 28, 2022
    Configuration menu
    Copy the full SHA
    9c1ccf2 View commit details
    Browse the repository at this point in the history
  2. review fixes

    MeirShpilraien committed Jul 28, 2022
    Configuration menu
    Copy the full SHA
    8fcbeb5 View commit details
    Browse the repository at this point in the history