Skip to content

Enhance Caching System and Optimize Memory Usage with Customizable Storage Support#1016

Merged
pieroit merged 3 commits intocheshire-cat-ai:developfrom
AlboCode:feature/cache
Feb 6, 2025
Merged

Enhance Caching System and Optimize Memory Usage with Customizable Storage Support#1016
pieroit merged 3 commits intocheshire-cat-ai:developfrom
AlboCode:feature/cache

Conversation

@AlboCode
Copy link
Contributor

Description

Add a caching system and optimize memory usage by removing stray_cats from the app state and storing the working memory in the cache.

Related to issue #846

Next, I would like to add a hook to customize the cache initialization, allowing to integrate the preferred storage, such as Redis, through a plugin.
I'm still unsure how to make the TTL of the working_memory customizable. Open to suggestions.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

@pieroit
Copy link
Member

pieroit commented Jan 25, 2025

Thanks @AlboCode this is an important feat.
Will review asap!

@pieroit pieroit changed the base branch from main to develop February 5, 2025 18:18
@pieroit pieroit merged commit 2dd3ac1 into cheshire-cat-ai:develop Feb 6, 2025
1 of 2 checks passed
@pieroit
Copy link
Member

pieroit commented Feb 6, 2025

@AlboCode great contribution thanks.
I adapted the naming to convention we have around in core and fixed the stray flow a little more.

Unfortunately there are mismatches between HTTP and WS strays in working memory, need to investigate a little more and write many tests.

Hopefully this is a step more to make the Cat stateless :D
Let me know when you are around for a little chat, I'd like to show you some cetriolis

If not I'll write them down in issues, I'm cooked

@AlboCode
Copy link
Contributor Author

AlboCode commented Feb 6, 2025

@pieroit Would be great to have a chat, unfortunately my time shift doesn't help 🥲
I'm online when in Italy is 1:00 AM until 2:00 PM

@pieroit
Copy link
Member

pieroit commented Feb 7, 2025

@pieroit Would be great to have a chat, unfortunately my time shift doesn't help 🥲 I'm online when in Italy is 1:00 AM until 2:00 PM

available tonight

@lucapiccinelli
Copy link

lucapiccinelli commented May 2, 2025

Hello @pieroit . What do you think if I continue on this PR, implementing a RedisCache? Like this
image

I perfectly agree with @AlboCode that the right vision is to have the cache manager pluggable. Despite this consideration, I would suggest having Redis hard-coded as a cache manager, in order to make CheshireCat a scalable solution out-of-the-box, without requiring any additional plugin.

I would argument further with some advantages and disadvantages that come to my mind:
Disadvantages:

  • add the Redis client dependency

Advantages:

  • For big numbers and for granting no downtime, scaling is not an option. Providing it natively, you also give a message.
  • No plugin to install, means less friction in the adoption of a solution. For example, you don't need a new build if you started in-memory. Or consider big structured teams where the infrastructure and the application development are owned by different teams. New plugin install may mean a new ticket that goes in someone else backlog.
  • Direct governance. Do you really want to move the governance of such an important feature outside of the core team?

I'm not trying to avoid a more complex development. My opinion is sincere. As a proof, If you agree, i will also provide a proposal for the plugin version of cache management.

What do you think?
Thank you, have a nice day.

@pieroit
Copy link
Member

pieroit commented May 2, 2025

I would argument further with some advantages and disadvantages that come to my mind: Disadvantages:

  • add the Redis client dependency

Advantages:

  • For big numbers and for granting no downtime, scaling is not an option. Providing it natively, you also give a message.
  • No plugin to install, means less friction in the adoption of a solution. For example, you don't need a new build if you started in-memory. Or consider big structured teams where the infrastructure and the application development are owned by different teams. New plugin install may mean a new ticket that goes in someone else backlog.
  • Direct governance. Do you really want to move the governance of such an important feature outside of the core team?

I'm not trying to avoid a more complex development. My opinion is sincere. As a proof, If you agree, i will also provide a proposal for the plugin version of cache management.

What do you think? Thank you, have a nice day.

Redis client as a dependency can be done, do you know if it works also with Valkey?
Another issue is related to the config, at the moment we are setting the cache via env variables, is that going to be enough? How many? If instead we need a JSON or YML that complicates things.

Also having hooks and REST API endpoints for setting the cache like we do with llms and auth seems overkill (I regret having the authhandler in the endpoints)

That's why for v2 I would go for a settings.py alà Django where you can declare system wide settings (cache, auth handler and models) without the limitation of an env notation (which can be in any case referenced from settings.py) and without the endpoints and dedicated factory.

So @lucapiccinelli I think it is worth it to have a redis cache in core, as long as:
1 - config can be limited to just a few env variables
2 - you're going to maintain it :)

Thyanks & Peace

@lucapiccinelli
Copy link

@pieroit Thank you for your answer. Yes, look like is it working also with Valkey.

1 - yes it is just something like this

redis_host = get_env("CCAT_REDIS_HOST")
redis_port = get_env("CCAT_REDIS_PORT")
redis_db = get_env("CCAT_REDIS_DB")

2 - I'm sorry, I really would like to be a maintainer of such a good project, but I can't commit to it.

Anyway, digging in the code, I found that it is already really simple to implement your cache manager in a plugin. It is as simple as this:

from cat.env import get_env
from cat.mad_hatter.decorators.hook import hook


@hook
def after_cat_bootstrap(cat):
    redis_host = get_env("CCAT_REDIS_HOST")
    redis_port = get_env("CCAT_REDIS_PORT")
    redis_db = get_env("CCAT_REDIS_DB")
    from cat.plugins.cc_redis_cache.redis_cache import RedisCache

    cat.cache = RedisCache(redis_host, redis_port, redis_db)
    print("setting redis cache bla")

I can still open a PR to add it to the core, but maintaining it is something that I can't do.

Despite that, I found a bug introduced in this commit in the file websocket.py, commenting the line 48. It makes a cleared conversation to be kept when you refresh the browser. If affects also the fylesystem_cache.
I will open a PR to fix it, with a clearer bug description and how to reproduce it.

I know that's not the right place, but as we are here, I add a working draft of the RedisCache implementation.

from cat.cache.base_cache import BaseCache
from cat.cache.cache_item import CacheItem

import pickle
import redis

class RedisCache(BaseCache):
    def __init__(self, host="localhost", port=6379, db=0):
        self._redis = redis.Redis(host=host, port=port, db=db)

    def insert(self, cache_item: CacheItem):
        encoded_item = pickle.dumps(cache_item)
        self._redis.set(
            cache_item.key,
            encoded_item,
        )

    def get_item(self, key) -> CacheItem:
        cache_item = self._get_and_decode(key)
        if not cache_item:
            return None

        if cache_item.is_expired():
            self.delete(key)
            return None

        return cache_item

    def get_value(self, key):
        cache_item = self._get_and_decode(key)
        if not cache_item:
            return None

        return cache_item.value

    def delete(self, key):
        print(f"clearing my conversation for {key}")
        self._redis.delete(key)

    def _get_and_decode(self, key) -> CacheItem | None:
        encoded_item = self._redis.get(key)
        if encoded_item:
            cache_item = pickle.loads(encoded_item)
            return cache_item
        else:
            return None

@lucapiccinelli
Copy link

I opened a PR to fix the cache bug #1076

@pieroit
Copy link
Member

pieroit commented May 12, 2025

Let's reiterate on the redis cache during v2 development.
My plan is to make it way easier to substitute pieces of the framework, without having to build a factory and endpoints for each ;)

Thanks

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.

3 participants