Replicate results of run commands instead of verbatim

[lantiga]

Adresses #84.
Replication of MODELRUN and SCRIPTRUN commands will not lead to re-executing the computation on the replicas, but just to setting the outputs on the replicas. Tests now include using replicas.

[lantiga]

Tests are passing locally (macOS), while CI is failing with a message

redis.exceptions.ResponseError: cannot get tensor from empty key

it appears that the replica doesn't contain the replicated output as it should.

Does this ring any bell at the RLTest level or else? @MeirShpilraien @mnunberg @rafie

[lantiga]

I could replicate the issue locally. It doesn't always fail, but it fails more often than not.

[lantiga]

Update on this. Forgetting about RLTest for a second, here's a breakdown of the issue.

First checkout the branch and build. Then start a master and a replica (make sure there's no rdb lingering around)

rm dump.rdb
redis-server --loadmodule build/redisai.so

and in another terminal

redis-server --port 7777 --replicaof 127.0.0.1 6379 --loadmodule build/redisai.so 

Now run

r = redis.Redis(host='localhost', port=6379)

model_filename = 'test/test_data/graph.pb'

with open(model_filename, 'rb') as f:
    model_pb = f.read()

ret = r.execute_command('AI.MODELSET', 'm', 'TF', 'CPU', 'INPUTS', 'a', 'b', 'OUTPUTS', 'mul', model_pb)

r.execute_command('AI.TENSORSET', 'a', 'FLOAT', 2, 2, 'VALUES', 2, 3, 2, 3)
r.execute_command('AI.TENSORSET', 'b', 'FLOAT', 2, 2, 'VALUES', 2, 3, 2, 3)

r.execute_command('AI.MODELRUN', 'm', 'INPUTS', 'a', 'b', 'OUTPUTS', 'c')

ret = r.execute_command('AI.TENSORGET', 'c', 'VALUES')
print(ret)

r2 = redis.Redis(host='localhost', port=7777)

ret = r2.execute_command('AI.TENSORGET', 'c', 'VALUES')
print(ret)

The script will fail at the last TENSORGET with

redis.exceptions.ResponseError: cannot get tensor from empty key

because there's no c on the replica.

The slave complains with

== CRITICAL == This replica is sending an error to its master: 'MULTI calls can not be nested' after processing the command 'multi'

In fact, logging from Redis internals, it looks like the master initiates the replica with the correct command args, but it only ends up sending MULTI and not the rest of the command.

If we repeat the above with SCRIPTRUN,

r = redis.Redis(host='localhost', port=6379)

script_filename = 'test/test_data/script.txt'

with open(script_filename, 'rb') as f:
    script = f.read()

r.execute_command('AI.SCRIPTSET', 'ket', 'CPU', script)

r.execute_command('AI.TENSORSET', 'a', 'FLOAT', 2, 2, 'VALUES', 2, 3, 2, 3)
r.execute_command('AI.TENSORSET', 'b', 'FLOAT', 2, 2, 'VALUES', 2, 3, 2, 3)

r.execute_command('AI.SCRIPTRUN', 'ket', 'bar', 'INPUTS', 'a', 'b', 'OUTPUTS', 'c')

ret = r.execute_command('AI.TENSORGET', 'c', 'VALUES')
print(ret)
    
r2 = redis.Redis(host='localhost', port=7777)

ret = r2.execute_command('AI.TENSORGET', 'c', 'VALUES')
print(ret)

There's no error and the replica contains the right value.

The difference is that MODELRUN is blocking, and we call Replicate(...) upon unblocking, from the callback we pass to the BlockClient function.

Even worse than this, it looks like replication is somewhat stuck. If we set another tensor at a new key d after MODELRUN and try to get it from the replica, we still can't find it.

r = redis.Redis(host='localhost', port=6379)

model_filename = 'test/test_data/graph.pb'

with open(model_filename, 'rb') as f:
    model_pb = f.read()

ret = r.execute_command('AI.MODELSET', 'm', 'TF', 'CPU', 'INPUTS', 'a', 'b', 'OUTPUTS', 'mul', model_pb)

r.execute_command('AI.TENSORSET', 'a', 'FLOAT', 2, 2, 'VALUES', 2, 3, 2, 3)
r.execute_command('AI.TENSORSET', 'b', 'FLOAT', 2, 2, 'VALUES', 2, 3, 2, 3)

r.execute_command('AI.MODELRUN', 'm', 'INPUTS', 'a', 'b', 'OUTPUTS', 'c')
r.execute_command('AI.TENSORSET', 'd', 'FLOAT', 2, 2, 'VALUES', 2, 3, 2, 3)

ret = r.execute_command('AI.TENSORGET', 'd', 'VALUES')
print(ret)

r2 = redis.Redis(host='localhost', port=7777)

ret = r2.execute_command('AI.TENSORGET', 'd', 'VALUES')
print(ret)

fails with

redis.exceptions.ResponseError: cannot get tensor from empty key

[gkorland]

https://github.com/antirez/redis/issues/6228

[lantiga]

@gkorland Now that the fix has been included in Redis unstable, should we wait for the next Redis release before merging this one? Or should we try to make things work in both cases by looking at the version of the Redis server? (if possible, I'm actually not sure how to do it)

[K-Jo]

@gkorland on which branches are the redis changes present? Will it be part of an upcoming 5 release or only be present in 6?

[gkorland]

@K-Jo should be on the new 5.0.7

[lantiga]

@rafie I wanted to test the fixes that ended up in Redis 5.0.7, and I rebased on master and changed Docker images to be based off 5.0.7, but apparently CircleCI is still on 5.0.5. I entered the CircleCII container and ran redis-server --version to find out:

Redis server v=5.0.5 sha=00000000:0 malloc=jemalloc-5.1.0 bits=64 build=442b43d467cd2b03

Can you give me a hint on how to have the container updated? Thanks a lot in advance.

[lantiga]

Tests are passing with 5.0.7 :-) @gkorland @rafie @K-Jo

[gkorland]

Great! just please make sure you set the redis version in the RAMP file to 5.0.7

[lantiga]

@gkorland @rafie it should be enough to change the version number in this line
https://github.com/RedisAI/RedisAI/blob/master/ramp.yml#L8
to 5.0.7 right?

[K-Jo]

@rafie @gkorland should this not be 5.4.11 ?

[K-Jo]

the pack version

[lantiga]

You tell me :-)

By the way, I see the license is also outdated. I can update both, we go with 5.4.11 then?