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

RDB compatibility with ReJSON #19

Closed
joehu21 opened this issue Dec 6, 2024 · 1 comment
Closed

RDB compatibility with ReJSON #19

joehu21 opened this issue Dec 6, 2024 · 1 comment
Assignees

Comments

@joehu21
Copy link
Collaborator

joehu21 commented Dec 6, 2024

Found a bug that RDB compatibility with ReJSON is broken. The root cause is that some ReJSON aux type processing code were missed out in the initial PR. The bug was not caught in the integration test because the RDB compatibility test is missing. So, we should fix the test gap as well.

Also discovered that we are only RDB compatible with ReJSON up to ReJSON v2.6.12. ReJSON has changed RDB format since v2.8. Created a separate issue: #20.

@joehu21 joehu21 self-assigned this Dec 6, 2024
joehu21 pushed a commit to joehu21/valkeyJSON that referenced this issue Dec 6, 2024
joehu21 pushed a commit to joehu21/valkeyJSON that referenced this issue Dec 6, 2024
joehu21 pushed a commit to joehu21/valkeyJSON that referenced this issue Dec 10, 2024
@joehu21 joehu21 reopened this Dec 17, 2024
@joehu21
Copy link
Collaborator Author

joehu21 commented Dec 17, 2024

I think we should just remove all aux type code not related to the JSON module, because when we talk about RDB compatibility, it is under the condition that no other module types are present. In addition, earlier releases of rejson docker images used to include search module, but newer rejson docker images no longer does that.

Appendix - Testing RDB compatibility with various releases of ReJSON docker images

The following is a test to explain why we need to load search aux types:

  1. Start ReJSON docker image:
docker run -p 6379:6379 --name redis-redisjson redislabs/rejson:2.0.6

Although it's a rejson image, it contains search module as well:

127.0.0.1:6379> module list
1) 1) "name"
   2) "ReJSON"
   3) "ver"
   4) (integer) 20006
2) 1) "name"
   2) "search"
   3) "ver"
   4) (integer) 20205
  1. We cannot unload the search module because it exports the search aux types:
127.0.0.1:6379> module unload search
(error) ERR Error unloading module: the module exports one or more module-side data types, can't unload
  1. create one json key:
json.set k1 . 1
  1. save
  2. copy out RDB:
docker cp $(docker ps -q):/data/dump.rdb dump.rdb
  1. load the RDB using ValkeyJSON binary that does not include PR Fix #19 - RDB compatibility with ReJSON and compatibility test #21. It failed:
22242:M 12 Dec 2024 02:50:45.568 * Loading RDB produced by Redis version 6.2.5
22242:M 12 Dec 2024 02:50:45.568 * RDB age 124 seconds
22242:M 12 Dec 2024 02:50:45.568 * RDB memory usage when created 0.92 Mb
22242:M 12 Dec 2024 02:50:45.568 # The RDB file contains AUX module data I can't load: no matching module 'ft_index0'

That's because code for loading aux type ft_index0 is missing.
6. load the RDB using ValkeyJSON binary that includes PR #21. It succeeded.

Note that although we have not created any search index, Redis still writes search aux types to RDB. I think the problem comes from RediSearch. It must have used aux_save instead of aux_save2. For aux_save, even if there is no module data (search index in this case), it writes aux type meta data to RDB. The correct way is to use aux_save2, which will not write module types if there is no module data. The fix should come from RediSearch. We can open an issue for them. If they fix the problem, then we do not need to load search aux types.

I just realized some of the rejson RDB test files, included in this PR, were generated using redis-stack docker images and that's why gears and graph aux types need to be registered in order to load them. This can be fixed by regenerating the RDBs using rejson docker images, and remove code related to loading grears & graph aux types.

However, there's still a question regarding search aux types. If we want to help migrating ReJSON to VakeyJSON, do we need to load aux search types?

I believe there are two search aux types - scdtype00 and ft_index00. Some ReJSON docker images contain RediSearch and some do not. e.g.,

  1. The following images contain RediSearch:
  • docker run -p 6380:6379 --name redis-redisjson redislabs/rejson:latest
  • docker run -p 6379:6379 --name redis-redisjson redislabs/rejson:2.0.6
  • any release of v2.0 or older
  1. The followings do not contain RediSearch (between v 2.2 and v2.6.12):
  • docker run -p 6379:6379 --name redis-redisjson redislabs/rejson:2.6.12
  • docker run -p 6379:6379 --name redis-redisjson redislabs/rejson:2.4.0
  • docker run -p 6379:6379 --name redis-redisjson redislabs/rejson:2.2.0
    (NOTE: we are not compatible with ReJSON v2.8 or later anyway. See ValkeyJSON is not RDB compatible with ReJSON 2.8 or later #20. So it's not listed here.)

For RDBs generated from docker images that do not contain RediSearch, we do not need to load search aux types. However, for RDBs generated from docker images that contain RediSearch, we need to load the search aux types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant