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

Add hubot brain storage module #72

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jimcortez
Copy link

Add new hubot_brain storage option to use built-in hubot persistence.

This makes it so you can use other plugins to persist hubot (like hubot-redis-brain) as a whole.

You may want to consider making this default.

@smashwilson
Copy link
Owner

This is setting the entire transition map into a single key-value pair within the brain's storage. The brain storage units that I've used (redis and postgresql) store that as a JSON blob in the associated backend. The markov model in the bot I run currently has about 2.5 million transition rows in its forward-chain model. Serializing and deserializing a JSON structure of that size on every received message would cause serious performance problems.

It's probably feasible to implement something like this backed by robot.brain.set and robot.brain.get with a key prefix instead. At least with the PostgreSQL brain variant I'm using, that would store each transition in a separate database row and do lookups with an indexed SELECT, which should be more scalable. (I'm not sure why I didn't do it that way to begin with - I think this module predates me finding/writing a PostgreSQL brain I was happy with?)

We should also alter the storage integration test suite to exercise this backend too.

@jimcortez
Copy link
Author

I actually use the redis integration of markov myself. I agree that in the long-term that having it in a separate store is probably wise. Just seemed like a good option to have for small bots, or for fun development.

You won't hurt my feelings if you close this PR.

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