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

Providing handlers for Data Persistence #45

Closed
wants to merge 3 commits into from

Conversation

balachandarsv
Copy link

Bot developers can connect to their favourite data stores now using this PR.

@alecl
Copy link

alecl commented Nov 22, 2017

@kirsle @marceloverdijk any thoughts on this PR? Externalizing persistence is a good core feature.

@marceloverdijk
Copy link
Member

@alecl do you think the proposed PersistenceHandler and PersistenceSessionManager really adds benefits? It's merely another - but very similar - abstraction layer around the already existing SessionManager. I personally think the current SessionManager (http://static.javadoc.io/com.rivescript/rivescript-core/0.10.0/com/rivescript/session/SessionManager.html) offers enough flexibility already.

That being said I still think that a other out-of-the-box session manager as proposed in #35 should have value.

Personally I think you have to be careful using persistent session manager.
RiveScript tends to multiple calls to the session manager so if you are not careful it will mean multiple database calls within a single transaction to get a reply. I think @kirsle wrote an article about it in the past.
I tend to retrieve the user vars from my persistent store (e.g. memcache) and feed it to the RiveScript instance and then get the reply. After I get the reply I extract the user vars from the RiveScript instance, store them again in my persistent store and remove them from the RiveScript instance. The RiveScript instance itself uses the ConcurrentHashMapSessionManager.
For me that gives the best performance; but for other users a different approach might be more efficient.

Also providing a generic database session manager might not be suitable for many users. Think about supporting various database vendors, or even how to define the database schema.

@alecl
Copy link

alecl commented Nov 22, 2017

It seemed like this PR was looking to come up with a tiered session storage abstraction with the "thaw" aspect. Maybe a write-through tiered cache is a more straightforward way to manage this though with Redis write-through to a relational db for safer longterm storage for example. Probably best to move further with #35 first to get some base implementations in.

@marceloverdijk
Copy link
Member

@alecl Yes I think going further with #35 and deliver some base implementations is the way ahead. If we can provide some generic implementation I would be very happy. PR's are welcome :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants