Redisearch: FT.SEARCH from Lua gives "ERR Blocking module command called from Lua script"

[tw-bert]

redis-cli examples below.

FT.CREATE "ftFoo" STOPWORDS 0 SCHEMA MyText TEXT NOSTEM

And then

FT.ADD "ftFoo" "ftKey01" 1.0 "FIELDS" "MyText" "the crazy fox jumps over the lazy dog"
FT.SEARCH "ftFoo" "crazy"

Correctly returns:

1) (integer) 1
2) "ftKey01"
3) 1) "MyText"
   2) "the crazy fox jumps over the lazy dog"

However, the following fails:

eval "return redis.call('FT.SEARCH','ftFoo','crazy')" 0
---
(error) ERR Error running script (call to f_8fd6bcceca8dddd4bea1e70b3dbfd076b7f12145): @user_script:1: ERR Blocking module command called from Lua script

Of course, we could live with a workaround like FT.SAFEADD , but I can't find a non-blocking alternative to FT.SEARCH . One thing I found is Query_SetConcurrentMode, but I'd probably have to dive into the code a bit more to be able to use it (plus, it's commented out). As another workaround, I thought there was a preprocessor I could use to disable any parallelism, but can't remember nor find where it's documented. Would that fix this scenario for the time being?

[dvirsky]

You can start the engine with SAFEMODE and it will work fine in Lua. Of course you'll lose concurrency, but it will work.

If you want to have some queries work concurrently and some not - that will require an extra argument in the query.

[tw-bert]

Great, just found it ;) . Can I pass SAFEMODE from the conf file?

[tw-bert]

The redis documentation isn't clear about the possibility of parameters to modules: https://github.com/antirez/redis/blob/unstable/redis.conf#L40

[tw-bert]

But, it's very clear in the documentation of RediSearch, never mind: http://redisearch.io/Configuring/

[tw-bert]

I can confirm that running with SAFEMODE resolves this issue, as predicted by @dvirsky:

# redis.conf:

################################## MODULES #####################################

# Load modules at startup. If the server is not able to load modules
# it will abort. It is possible to use multiple loadmodule directives.
#
# loadmodule /path/to/my_module.so
# loadmodule /path/to/other_module.so
# RediSearch Parameters: see http://redisearch.io/Configuring/
# SAFEMODE is added because of: https://github.com/RedisLabsModules/RediSearch/issues/249
loadmodule /usr/local/bin/RediSearch/1.0.2.dmd-improvements/redisearch.so SAFEMODE

server log:

17931:C 27 Dec 14:17:09.070 # oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
17931:C 27 Dec 14:17:09.071 # Redis version=4.0.6, bits=64, commit=00000000, modified=0, pid=17931, just started
17931:C 27 Dec 14:17:09.071 # Configuration loaded
17933:M 27 Dec 14:17:09.072 * Increased maximum number of open files to 10032 (it was originally set to 1024).
17933:M 27 Dec 14:17:09.074 * Running mode=standalone, port=14213.
17933:M 27 Dec 14:17:09.074 # Server initialized
17933:M 27 Dec 14:17:09.075 * <ft> Configuration: concurrent mode: 0, ext load: (null), min prefix: 2, max expansions: 200,
17933:M 27 Dec 14:17:09.094 * <ft> Initialized thread pool!
17933:M 27 Dec 14:17:09.094 * Module 'ft' loaded from /usr/local/bin/RediSearch/1.0.2.dmd-improvements/redisearch.so

Test again:

127.0.0.1:14213> eval "return redis.call('FT.SEARCH','ftFoo','crazy')" 0

1) (integer) 1
2) "ftKey01"
3) 1) "MyText"
   2) "the crazy fox jumps over the lazy dog"

I'll leave this issue open, since I feel that this behaviour should be client context agnostic (as soon as RediSearch can rely on getting the Lua context from within the hook, and enough users have upgraded to Redis 4.0.6+, as discussed before). For now, we're out of the blue again.

[tw-bert]

@dvirsky Hmm, cheered too soon. Apparently SAFEMODE causes panic. A query like

FT.SEARCH "~ftidx.lo.someindex" "@somenumericfield:[(0 +inf]" LIMIT 0 10

Causes huge CPU load for a few seconds and then crashes the server. This happens from Lua as well as directly from redis-cli.

I just restarted without SAFEMODE, now the same query runs fine again (outside of Lua).

Log snippet:

Fast memory test PASSED, however your memory can still be broken. Please run a memory test for several hours if possible.

------ DUMPING CODE AROUND EIP ------
Symbol: ConcurrentSearch_AddKey (base: 0x7f1e04abfc40)
Module: /usr/local/bin/RediSearch/1.0.2.dmd-improvements/redisearch.so (base 0x7f1e04a70000)
$ xxd -r -p /tmp/dump.hex /tmp/dump.bin
$ objdump --adjust-vma=0x7f1e04abfc40 -D -b binary -m i386:x86-64 /tmp/dump.bin
------
26196:M 27 Dec 14:58:30.965 # dump of function (hexdump of 159 bytes):
415741564989ff415541544d89c555534889f589d34989cc4d89ce4883ec088b47288d7001897728488d04f500000000488b7f2048c1e6064829c6e840d6fdff418b5728498947208d4aff488d14cd0000000048c1e1064829d1488d340831c0b907000

=== REDIS BUG REPORT END. Make sure to include from START to END. ===

[tw-bert]

@dvirsky I managed to fix the numeric index issue with SAFEMODE enabled.

https://github.com/RedisLabsModules/RediSearch/blob/422eb866e6f6265c984d52b33f062bf404bafab9/src/numeric_index.c#L373-L374

surrounded it with:

  if (csx) {
    ...
  }

(please verify & patch)

So for now it seems we have the bare essentials of RediSearch for our use cases, and can start experimenting with it.

[dvirsky]

@tw-bert can you open a PR for that?

[mnunberg]

Can confirm repro & fix.

[tw-bert]

@dvirsky We don't use git internally (hg instead), so I just monkeypatched it... no PR I'm afraid

[dvirsky]

well @mnunberg fixed it. But you're contributing so much, it would be cool to see you on the committer list @tw-bert :)

[tw-bert]

Thank you for your kind words :) If I have something to contribute which involves more than a null check, I'll do a proper PR (possibly the big endian support, if I get around to finish it. Sidenote: using hg there, with hg-git extension).

[dvirsky]

@tw-bert FYI actually for such small fixes, you can just create the PR inside github's website. Just go to a file, hit "edit", make your changes and save them - and it will suggest creating a PR for you.

[mnunberg]

Fixed by 8278f64e95355e18a6bbe1259cc435981468f3fc

[tw-bert]

@mnunberg This issue isn't closed I think. RediSearch needs to take the non-parallel route when it detects a Lua context. We applied SAFEMODE as a workaround, and made a patch to get that workaround working, but of course SAFEMODE is hurting performance for non-Lua contexts.

[dvirsky]

@tw-bert you're right. Since 4.0.6 knows that, we can now safely do it. I'll open a separate issue.

[dvirsky]

@tw-bert BTW have you read my email?

[kchristidis]

If you want to have some queries work concurrently and some not - that will require an extra argument in the query.

Apologies for reviving an old thread but does this suggest that one can have some queries issues concurrently and others serially? If so, how can this be configured at runtime?

My understanding is that the instance has to be booted with SAFEMODE in order to disable concurrency, and this is not a toggle that can be switched on a per-call basis. Let me know if I'm wrong.