Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

AI.INFO command #259

Merged
merged 4 commits into from
Feb 3, 2020
Merged

AI.INFO command #259

merged 4 commits into from
Feb 3, 2020

Conversation

lantiga
Copy link
Contributor

@lantiga lantiga commented Dec 26, 2019

This PR adds a new AI.INFO command to RedisAI.
Stats on each model/script key being run are accumulated and reported periodically.

TODO items:

  • refactor with modelkey as a mandatory argument (details below in the UPDATE)
  • finalize what info gets returned (added ERRORS, evaluating HISTOGRAM)
  • report batch size per call
  • add info reset command (AI.INFO modelkey RESET) to clean up all stat keys and start fresh
  • write tests
  • write docs

UPDATE (credit @K-Jo): avoid getting stats from all models with a single command (response potentially unbounded, it looks like a scan), but have users call AI.INFO with a modelkey as the argument. This way a) the stats will have the same lifetime as the respective models (we can collocate stats directly in the model struct and get rid of the module-level dict UPDATE: we will still keep the stats in the dict to avoid accessing keyspace in the callback); b) responses will be bounded and specific for each model. Users will also be in charge of calling RESETSTAT on the model stats.

@lantiga lantiga self-assigned this Dec 26, 2019
@lantiga lantiga mentioned this pull request Dec 26, 2019
@filipecosta90
Copy link
Collaborator

@lantiga @K-Jo @gkorland OSS look and feel. I think that @itamarhaber can be of extreme help here

https://redis.io/commands/config-resetstat
https://redis.io/commands/info

@filipecosta90 filipecosta90 self-requested a review January 2, 2020 10:48
src/redisai.c Outdated Show resolved Hide resolved
src/redisai.c Outdated Show resolved Hide resolved
src/redisai.c Show resolved Hide resolved
@lantiga
Copy link
Contributor Author

lantiga commented Jan 25, 2020

Added tests and docs, the PR is ready to land, except for the histograms.
@filipecosta90 please let me know what you'd like to do regarding that (see above in the comments). Thanks!

@lantiga lantiga changed the title AI.INFO command [WIP] AI.INFO command Jan 25, 2020
### AI.INFO Example

```sql
AI.INFO amodel
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lantiga can we include the example output here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@lantiga lantiga merged commit b569fce into master Feb 3, 2020
lantiga added a commit that referenced this pull request May 6, 2020
 
AI.INFO command (#259)

* Add AI.INFO command

* Add tests

* Add docs for AI.INFO

* Added output to AI.INFO example
@gkorland gkorland deleted the info branch October 6, 2020 08:57
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.

None yet

2 participants