Skip to content

Conversation

alonre24
Copy link
Collaborator

Support async LLAPI (part 2/3): Refactor Script run command (parsing, execution as a single DAG op, persist tensors before reply to client). Support async LLAPI, in a similar way to the new model run async LLAPI.

@alonre24 alonre24 self-assigned this Dec 28, 2020
@codecov
Copy link

codecov bot commented Dec 28, 2020

Codecov Report

Merging #547 (0b10fab) into master (a191e32) will increase coverage by 0.17%.
The diff coverage is 89.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #547      +/-   ##
==========================================
+ Coverage   75.84%   76.01%   +0.17%     
==========================================
  Files          25       25              
  Lines        5406     5508     +102     
==========================================
+ Hits         4100     4187      +87     
- Misses       1306     1321      +15     
Impacted Files Coverage Δ
src/redisai.h 0.00% <0.00%> (ø)
src/run_info.c 69.95% <81.81%> (+0.20%) ⬆️
src/script.c 64.82% <88.46%> (-1.13%) ⬇️
src/command_parser.c 90.62% <90.62%> (-0.08%) ⬇️
src/DAG/dag.c 87.14% <92.85%> (+0.87%) ⬆️
src/DAG/dag_parser.c 87.50% <100.00%> (-2.15%) ⬇️
src/model.c 69.55% <100.00%> (+0.09%) ⬆️
src/modelRun_ctx.c 83.58% <100.00%> (+0.97%) ⬆️
src/redisai.c 80.52% <100.00%> (+0.55%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a191e32...0b10fab. Read the comment docs.

…e runkey in DagOp free function. Closing the model key no longer leeds to a crash.
int res;
for (size_t i = 0; i < len; i++) {
res = Script_RunCtxAddParam(sctx, &sctx->inputs, inputTensors[i]);
res = _Script_RunCtxAddParam(&sctx->inputs, inputTensors[i]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH
The function always returns 1 so maybe it should return void?

Comment on lines +209 to +213
if (!strcasecmp(arg_string, "TIMEOUT")) {
if (_parseTimeout(argv[++argpos], error, timeout) == REDISMODULE_ERR)
return REDISMODULE_ERR;
arg_string = RedisModule_StringPtrLen(argv[++argpos], NULL);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please verify that this is not handled also in the dag parsing

@alonre24 alonre24 merged commit 34ee494 into master Dec 29, 2020
@alonre24 alonre24 deleted the Script_run_refactor_and_LLAPI branch December 29, 2020 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants