resty-redis-cluster icon indicating copy to clipboard operation
resty-redis-cluster copied to clipboard

Bug in fetch_slots - slot_cache is modified unexpectedly

Open ivanastepanovic opened this issue 1 year ago • 4 comments

There might be a bug in case that at some point in time redis is down and connection cannot be established.

In method fech_slots there is following line:

serv_list_combined = serv_list_cached.serv_list

which means serv_list_combined and serv_list_cached.serv_list and slot_cache[self.config.name .. "serv_list"].serv_list are all referencing the same table. Later when serv_list_combined is appended, slot_cache is affected and appended too (the same table!), which makes it longer and longer after each request, in case that redis is down. (On each request, slot_cached is filled with serv_list from config).

Possible solution could be to make serv_list_combined as a copy of serv_list_cached.serv_list:

serv_list_combined = {}
for _, value in ipairs(serv_list_cached.serv_list) do
   table.insert(serv_list_combined, value)
end

Please check. Thanks in advance! :)

ivanastepanovic avatar Nov 23 '23 14:11 ivanastepanovic

how can I reproduce this bug? Make Redis down and restart during reproduce?

tzssangglass avatar Nov 24 '23 03:11 tzssangglass

  1. Remove config param max_connection_timeout (in order to see how many slots are being tried)
  2. Make Redis up and running
  3. Establish connection successfully
  4. Make Redis down
  5. Try to connect multiple times
  6. See that on each request, number of slots that are tried in method try_hosts_slots is always increased by serv_list

Please let me know if more details are needed.

ivanastepanovic avatar Nov 24 '23 07:11 ivanastepanovic

since this repo is not active, I fixed this bug in the forked repo: https://github.com/Kong/resty-redis-cluster/pull/28, the new version will be released soon and can be found here: https://luarocks.org/modules/kong/kong-redis-cluster

thank you to the original author and contributors again.

tzssangglass avatar Dec 04 '23 08:12 tzssangglass