-
Notifications
You must be signed in to change notification settings - Fork 8
feat: adding support for session caching for redis and dynamodb session stores for better performance #170
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
Conversation
…on stores for better performance Signed-off-by: Tharindu Dissanayake <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces an optional in-memory LRU caching layer for Redis and DynamoDB session stores to improve performance by reducing backend storage lookups. The cache is configurable via a cache parameter in the session configuration, which specifies the maximum number of sessions to keep in memory.
Key changes:
- Implemented SessionCache class using OrderedDict with LRU eviction policy
- Modified RedisSessionStore and DynamoDBSessionStore to accept and use an optional SessionCache
- Added cache configuration parameter to session store config with default value of 0 (disabled)
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| ak-py/src/agentkernel/core/session/base.py | Added SessionCache class implementing LRU cache using OrderedDict |
| ak-py/src/agentkernel/core/session/redis.py | Integrated SessionCache into RedisSessionStore for load/store/clear operations |
| ak-py/src/agentkernel/core/session/dynamodb.py | Integrated SessionCache into DynamoDBSessionStore for load/store/clear operations |
| ak-py/src/agentkernel/core/config.py | Added optional cache configuration parameter to session store config |
| ak-py/src/agentkernel/core/builder.py | Modified SessionStoreBuilder to create SessionCache instance based on config and inject into stores |
| ak-py/tests/test_session.py | Added unit tests for SessionCache LRU eviction behavior |
| ak-py/tests/test_runtime.py | Updated test to include cache configuration parameter |
| examples/memory/redis/config.yaml | Added cache configuration example with value of 256 |
| examples/memory/dynamodb/config.yaml | Added cache configuration example with value of 256 |
Comments suppressed due to low confidence (1)
ak-py/src/agentkernel/core/session/dynamodb.py:217
- After loading a session from DynamoDB storage (lines 212-217), the session is not added to the cache. This means subsequent loads for the same session will always hit DynamoDB instead of using the cached value, defeating the performance optimization purpose of this PR.
The loaded session should be cached before returning it to ensure cache consistency and improve performance.
keys = self._driver.query_keys(session_id)
if not keys:
if strict:
raise KeyError(f"Session {session_id} not found")
self._log.warning("Session %s not found, creating new session", session_id)
return self.new(session_id)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Tharindu Dissanayake <[email protected]>
Signed-off-by: Tharindu Dissanayake <[email protected]>
Signed-off-by: Tharindu Dissanayake <[email protected]>
Signed-off-by: Tharindu Dissanayake <[email protected]>
Signed-off-by: Tharindu Dissanayake <[email protected]>
Signed-off-by: Tharindu Dissanayake <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Tharindu Dissanayake <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…in builder Signed-off-by: Tharindu Dissanayake <[email protected]>
Signed-off-by: Tharindu Dissanayake <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Signed-off-by: Tharindu Dissanayake <[email protected]>
Signed-off-by: Tharindu Dissanayake [email protected]