etaty / rediscala

Non-blocking, Reactive Redis driver for Scala (with Sentinel support)
Apache License 2.0
790 stars 142 forks source link

#44 Sort is a master only command #75

Closed javierarrieta closed 9 years ago

javierarrieta commented 9 years ago

SORT redis command is master only, so we need to configure the case class for that command accordingly

coveralls commented 9 years ago

Coverage Status

Coverage increased (+0.25%) to 91.95% when pulling d15dd3c9d07d8b88e4b2698900c1292a3c66a6f8 on javierarrieta:bugfix/#44 into 790d76411acaca20ebde49e9a6369b1c6ecfceae on etaty:master.

coveralls commented 9 years ago

Coverage Status

Coverage increased (+0.25%) to 91.95% when pulling d15dd3c9d07d8b88e4b2698900c1292a3c66a6f8 on javierarrieta:bugfix/#44 into 790d76411acaca20ebde49e9a6369b1c6ecfceae on etaty:master.

etaty commented 9 years ago

Hi, Could you explain why ? because Sort is read-only, so unless there is a big replication lag for the slave, it should be fine

javierarrieta commented 9 years ago

Hi, it is not read only, it writes the sorted elements in the list. It is surprising that it is not clear in the reference doc, but you can easily reproduce issuing that command against a slave

etaty commented 9 years ago

There is the rediscala sort store command

etaty commented 9 years ago

I have spitted the command, Sort and SortStore https://github.com/etaty/rediscala/blob/ab29ee24b024b8dced58f618d6a32bb91ba91bf2/src/main/scala/redis/api/Keys.scala#L144-L166

javierarrieta commented 9 years ago

Yeah, I don't understand too much the difference since both are issuing a redis SORT command that will rewrite the list to the key. We are using the sort command, not the sortStore

etaty commented 9 years ago

sortstore will read, then write (store) sort will only read

etaty commented 9 years ago

you can't store with the rediscala sort you have to use sortStore

javierarrieta commented 9 years ago

Sorry, but redis sort command writes to the key, as said it is quite easy to reproduce manually. I have been looking in the rediscala tests to see if I could easily make it apparent in those

etaty commented 9 years ago

I have splitted the redis sort command in rediscala

javierarrieta commented 9 years ago

And again I don't know how that could be beneficial as SORT command writes to the key

127.0.0.1:16379> lrange mylist 0 -1
1) "3"
2) "4"
3) "2"
4) "1"
127.0.0.1:16379> sort mylist
(error) READONLY You can't write against a read only slave.

I can solve hopefully my issue using sortStore, but anyone trying to use sort would have this issue (and could be bad if they introduce master-slave after deploying to prod, so code failing in production)

etaty commented 9 years ago

Oh ok never saw that error before, I will need to check it

javierarrieta commented 9 years ago

Cool, thanks @etaty