Refactor support for lua/multi on single slot indexes#866
Refactor support for lua/multi on single slot indexes#866bandalgomsu wants to merge 2 commits intovalkey-io:mainfrom
Conversation
Signed-off-by: Su Ko <rhtn1128@gmail.com>
| CONTROLLED_BOOLEAN(ForceReplicasOnly, false); | ||
| DEV_INTEGER_COUNTER(stats, single_slot_queries); | ||
|
|
||
| bool IsSingleSlotQueryRoutedToLocalNode(ValkeyModuleCtx *ctx, |
There was a problem hiding this comment.
Let's not recompute whether this Index is single-slot on every query access. Rather, IndexSchema should have an "std::optional<uint16_t> single_slot_number" that is setup when the index is created (or recreated from an rdb load). If that optional is populated, then it will be the slot number for the single-slot index.
There was a problem hiding this comment.
thanks for good suggestion 👍
I will apply it.
…en index is created or recreated Signed-off-by: Su Ko <rhtn1128@gmail.com>
| !parameters->local_only; | ||
|
|
||
| if (ABSL_PREDICT_FALSE(inside_multi_exec && do_fanout)) { | ||
| if (ABSL_PREDICT_FALSE(inside_multi_exec && do_fanout) && |
There was a problem hiding this comment.
Can you please remind me why MULTI/EXEC or Lua script are not supported in CME mode?
There was a problem hiding this comment.
Deadlock/Timeout. If node A fans out a request to node B and simultaneously node B sends a request to node A. then both A & B have mainthreads that are stuck in LUA and can't service the content requests from remote.
There is a solution to this problem, which I think we should consider, which is that when executing a fanout from within a LUA or multi that instead of doing the traditional blocking (which causes a busy loop in the core) that we actually do our own busy-waiting, and explicitly allow remote content fetching queries while in that state.
This eliminates the deadlock, but exposes the local database in the middle of a LUA/multi that isn't normally visible. But since you're issuing the query, you're making it visible at that point. So I think the query problem is simply a documentation issue.
I think this should be put on the queue as a feature for the next release.
3 participants
Enabled FT.SEARCH/FT.AGGREGATE in MULTI/EXEC and Lua for CME only when the index is single-slot (hash-tagged) and the current node owns that slot
issue : #798