apache / kvrocks

Apache Kvrocks is a distributed key value NoSQL database that uses RocksDB as storage engine and is compatible with Redis protocol.
https://kvrocks.apache.org/
Apache License 2.0
3.57k stars 468 forks source link

Inconsistent behavior with Redis in JSON.MSET command #2552

Closed VIVALXH closed 1 month ago

VIVALXH commented 1 month ago

Search before asking

Version

redis: image: redislabs/rejson:latest

kvrocks: image: apache/kvrocks:latest

Minimal reproduce step

redis: image: redislabs/rejson:latest

❯ redis-cli -h 192.168.1.72 -p 6369
192.168.1.72:6369> flushdb
OK
192.168.1.72:6369> JSON.SET doc $ '{"f1":{"a1":0},"f2":{"a2":0}}'
OK
192.168.1.72:6369> JSON.GET doc
"{\"f1\":{\"a1\":0},\"f2\":{\"a2\":0}}"
192.168.1.72:6369> JSON.MSET doc $.f1.a1 1 doc $.f2.a2 2
OK
192.168.1.72:6369> JSON.GET doc
"{\"f1\":{\"a1\":1},\"f2\":{\"a2\":2}}"

kvrocks: image: apache/kvrocks:latest

❯ redis-cli -h 192.168.1.72 -p 6666
192.168.1.72:6666> flushdb
OK
192.168.1.72:6666> JSON.SET doc $ '{"f1":{"a1":0},"f2":{"a2":0}}'
OK
192.168.1.72:6666> JSON.GET doc
"{\"f1\":{\"a1\":0},\"f2\":{\"a2\":0}}"
192.168.1.72:6666> JSON.MSET doc $.f1.a1 1 doc $.f2.a2 2
OK
192.168.1.72:6666> JSON.GET doc
"{\"f1\":{\"a1\":0},\"f2\":{\"a2\":2}}"

What did you expect to see?

I hope that the behavior of JSON.MSET in KVROCKS is consistent with Redis.

What did you see instead?

For Redis, the JSON.MSET command can modify multiple fields of the same key simultaneously, but for KVROCKS, only one field is successfully modified.

Anything Else?

I found that in the code, JSON.MSET performs a query for each sub-command, resulting in a separate result in memory for each key if the keys are the same. Can it be modified so that there is only one copy of the key in memory and no repeated reads? https://github.com/apache/kvrocks/blob/f2bc224bc342f7e2b679cb2dc79d7a31067643ca/src/types/redis_json.cc#L579C15-L579C17

No response

Are you willing to submit a PR?

git-hulk commented 1 month ago

@VIVALXH Good catch, would you like to fix this?

VIVALXH commented 1 month ago

Thank you for your response and for recognizing the issue. I'd be happy to try and fix this issue, and I plan to start working after my workday.

git-hulk commented 1 month ago

@VIVALXH Thank you!

PragmaTwice commented 1 month ago

Fixed in #2579.