Skip to content

Add relevant RedisAI config entries to the INFO output ( stage 1 ) #396

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Jul 23, 2020

Conversation

filipecosta90
Copy link
Collaborator

@filipecosta90 filipecosta90 commented Jun 1, 2020

Given that redis 6 is now GA, this PR:

  • Updates redismodule.h from upstream (using 6.0.0 version )
  • Adds relevant RedisAI config entries to the INFO output, so that standard monitoring systems would be able to monitor them.

This change is backward compatible.

Example output of info modules:

$ redis-cli info modules
# Modules
module:name=ai,ver=999999,api=1,filters=0,usedby=[],using=[],options=[]

# ai_git
ai_git_sha:510d3ce6a59821632729a43e8d4832ae9bfd2b2d

# ai_load_time_configs
ai_threads_per_queue:1
ai_inter_op_parallelism:0
ai_intra_op_parallelism:0

@lantiga WDYT about also adding the loaded backends and versions?
@MeirShpilraien this opens the door for standardizing the common parts across modules ( at least load time confis and git info should be transversal to all modules ) - WDYT?

… standard monitoring systems would be able to monitor them.
@codecov
Copy link

codecov bot commented Jun 1, 2020

Codecov Report

Merging #396 into master will increase coverage by 0.28%.
The diff coverage is 90.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #396      +/-   ##
==========================================
+ Coverage   72.97%   73.26%   +0.28%     
==========================================
  Files          21       21              
  Lines        4504     4579      +75     
==========================================
+ Hits         3287     3355      +68     
- Misses       1217     1224       +7     
Impacted Files Coverage Δ
src/redisai.c 77.97% <22.22%> (-0.82%) ⬇️
src/redismodule.h 100.00% <100.00%> (ø)

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 6a14c04...31fdb03. Read the comment docs.

Copy link
Contributor

@lantiga lantiga left a comment

Choose a reason for hiding this comment

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

This is good for now. Moving forward we should think about how to populate with eg backend info, etc, or runtime info, as you suggest.

Since we are loading backends dynamically, we need the backends to signal their versions (TF, PyTorch, ONNXRuntime) and build-time configuration for the backend (GPU, etc). We don't have such an API at this time, but I have been thinking this is needed anyway.

@filipecosta90 filipecosta90 changed the title Add relevant RedisAI config entries to the INFO output WIP Add relevant RedisAI config entries to the INFO output Jun 3, 2020
@filipecosta90 filipecosta90 changed the title WIP Add relevant RedisAI config entries to the INFO output Add relevant RedisAI config entries to the INFO output ( stage 1 ) Jul 22, 2020
@filipecosta90 filipecosta90 self-assigned this Jul 22, 2020
@filipecosta90 filipecosta90 merged commit 544e36e into master Jul 23, 2020
@filipecosta90 filipecosta90 deleted the modulesapi.info branch July 23, 2020 13:59
filipecosta90 added a commit that referenced this pull request Aug 24, 2020
… standard monitoring systems would be able to monitor them. (#396)
@filipecosta90 filipecosta90 mentioned this pull request Aug 24, 2020
DvirDukhan pushed a commit that referenced this pull request Sep 1, 2020
… standard monitoring systems would be able to monitor them. (#396)
filipecosta90 added a commit that referenced this pull request Sep 2, 2020
* Introduced readies submodule (#377)

* Introduced readies submodule

* Fix in paella

* Updated get_deps.sh and docs

* [WIP] add C API reference to docs using auto tool  (#368)

* [add] add C API reference to docs using auto tool ( python3 generate_llapi_reference.py )

* [add] generating C API reference on the fly

* [add] moving api_reference under Developer Nav

Co-authored-by: Luca Antiga <luca.antiga@orobix.com>

* [WIP] Enable AI.SCRIPTRUN on AI.DAGRUN* (#383)

* [add] decoupled scriptrun command parsing from runtime scriptrun.

* [add] split positive/negative tests on pytorch scriptrun.

* [add] refactor AI.DAGRUN_RO and AI.DAGRUN to use the same code base (with read/write modes)

* [add] added positive and negative tests for dagrun with scriptrun

* [add] updated documentation to reflect scriptrun support on dagrun

* [add] added example enqueuing multiple SCRIPTRUN and MODELRUN commands within a DAG

* Add support for variadic arguments to SCRIPT (#395)

* Add support for variadic arguments to SCRIPT

* Add negative errors

* atomic ref count (#403)

* Multi-platform build (#398)

* CircleCI: increased GPU test timeout to 40m (#404)

* CI: platform-build-defs -> on-master-and-version-tags (#405)

* Llapi updates (#400)

* added low level api return redis types

* added variadic to llapi

* fixed memory issue for params array re-alloc

* Avoid splitting outputs in batches when nbatches == 1 (#406)

* Avoid splitting outputs in batches when nbatches == 1

* Add batch size checks

* Fix batch checks

* Update readies

* Add bad batching test

* Update README.md

* CircleCI: fixed redis installation in coverage builds (#423)

* Docker6.0.5 (#427)

* Update Dockerfile

* Update Dockerfile.arm

* Update Dockerfile.gpu

* submodule pull moved up (#428)

* Update mkdocs.yml (#432)

* [add] Add relevant RedisAI config entries to the INFO output, so that standard monitoring systems would be able to monitor them. (#396)

* Updated support email and Orobix to Tensorwork on ramp file (#436)

* [add] updated support email and Orobix to Tensorwork on ramp file

Co-authored-by: Sherin Thomas <sherin@tensorwerk.com>

* CircleCI: Fixed problem with GPU testing (#440)

* Fixed platforms build problem (#441)

* Fixed platforms build problem

* Build: updated Redis versions

* Build: another Redis version update

* Dependencies in ramp.yml (#444)

* Fixed flagged as "getkeys-api" during the registration ( AI.DAGRUN, AI.DAGRUN_RO, AI.MODELRUN, AI.SCRIPTRUN ) (#438)

* [wip] wip on fixing the commands being flagged as getkeys-api during the registration. ( AI.MODELRUN, AI.SCRIPTRUN, AI.DAGRUN, AI.DAGRUN_RO )

* [add] pytorch oss cluster tests passing

* [add] TensorFlow lite tests passing on oss cluster

* [add] ONNX tests passing on oss cluster

* [add] TensorFlow tests passing on oss cluster

* [wip] wip on dag

* [add] DAG tests passing on oss cluster

* [add] enabling oss cluster tests on CI

* [add] bumping rmbuilder version from 6.0.1 to 6.0.5 on circleci

* [fix] fixed potential invalid mem accesses on RedisAI_DagRunSyntaxParser, RAI_parseDAGPersistArgs

* [fix] fixed RAI_FreeDagOp wrongfully calling RedisModule_Free on dagOp->inkeys, dagOp->outkeys

* [fix] fixed dictKey allocation on RAI_parseDAGLoadArgs

* [add] alter Sanitizer tests to accommodate keys on the same slot.

* [add] extracted the methods to respond to RedisModule_IsKeysPositionRequest() from main code of dagrun,modelrun, and scriptrun to specific methods

* [fix] Fixes per PR review

* [fix] fixes per PR review

* [fix] fix per CI ascii conversion error on test_dagro_modelrun_financialNet_no_writes_multiple_modelruns

* Safely add to arrays + fix for #443 (#449)

* Make sure we reassign the pointer in array_append

* Fix case-sensitive comparison for devicestr

* Fix sanitizer tests

(cherry picked from commit 5c7813e)

* fixed tests messages

* Shallow copy persisted tensor

* Resolve log format warnings

* Fix error message and test

* Fix memory management in local context dict

* Fixed artifacts handling + added tests logs aggregation (#445)

* Snapshot packages are placed into http://redismodules.s3.amazonaws.com/redisai/snapshots. Private branches (i.e., non-master) are also supported, just need to enable deploy-snapshot in config.yml to fire on on-any-branch rather than just on-master.
* Release packages are placed into http://redismodules.s3.amazonaws.com/redisai, and it's going to be crowded in there, so we may want to consider putting each version into its own directory.
* `BB` in-source breakpoints are now supported (in `DEBUG=1` builds): put `BB;` inside the code and it will stop if you're running under gdb.
* Tests results in CircleCI are now aggregated as artifacts.

* Build: setuptools-related fix (#455)

(cherry picked from commit d1fcd0d)

Co-authored-by: Rafi Einstein <raffapen@outlook.com>
Co-authored-by: Luca Antiga <luca.antiga@orobix.com>
Co-authored-by: DvirDukhan <dvir@redislabs.com>
Co-authored-by: Guy Korland <gkorland@gmail.com>
Co-authored-by: Sherin Thomas <sherin@tensorwerk.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants