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

Delete of Serverside objects when key is regenerated #323

Closed
Lxstr opened this issue May 6, 2024 · 6 comments
Closed

Delete of Serverside objects when key is regenerated #323

Lxstr opened this issue May 6, 2024 · 6 comments

Comments

@Lxstr
Copy link

Lxstr commented May 6, 2024

As described in the docs using Serverside() creates a new key for the data each time a callback is run. However, the old piece of data remains in the redis storage until it expires or redis fills up and starts emptying itself. The issue is that redis may start emptying data of other users whos data has not expired. You also can't tell if you genuinly do need more redis storage to accomodate a greater number of users.

The whole issue would be solved if it's possible to automatically delete the old key/value when a new key is generated.

I would be happy to PR but I can't find where this code is implemented.

@matt-sd-watson
Copy link

This looks like it's similar to my previous issue: #302

In short, you should be able to overwrite the previous key if you keep the key string consistent for all of the callbacks where the same Serverside object is returned, like this:

return Serverside(data, key="constant_key")

Keeping the string key constant will produce one constant pickle object that is overwritten any time a callback updates the particular Serverside object.

@Lxstr
Copy link
Author

Lxstr commented May 6, 2024

Hi Matt thanks for the idea, is that still going to use a unique key for different sessions/users? Or will it literally use that key for every user

@matt-sd-watson
Copy link

I wouldn't think about users so much as callback invocations. So for example, you could set up your application so that multiple users will access one instance of your application, so there will be multiple user requests for a session instance. In that case, you should not use the constant key and let the keys uniquely generate each time the callback is invoked, because otherwise the application will overwrite another user's data with the newest callback invocation, potentially from a different user. I don't see a way around this if multiple users will share an instance/session.

Conversely, you could somehow enforce that each user gets a separate instance (i.e. each user gets a new containerized application), and in that case you can use the constant keys because you know that only one user is invoking the callbacks for a particular session.
In my application, we have the CLI option to toggle this on or off.

@marcstern14
Copy link

I am dealing with trying to understand the implications of fixed keys as well (#314). In my application, I have been using fixed keys mostly successfully, which makes me think that each user is invoking a unique session. However, because I am using Serverside() I often experience "missing data" if I open up the website the next day, and need to refresh to get the data to load. Any thoughts on this behavior @matt-sd-watson? Is there a clever way to implement FileSystemStorage cleanup without running a separate job since my application lives inside a docker container?

@Lxstr
Copy link
Author

Lxstr commented Jun 14, 2024

So a proposed solution for different users (also more secure than local host maybe for static keys?):

Setting the storage with Serverside(data, session={lambda: session.sid}, key="data-store") on a component with id="data-store" would set a cache key of "{session.id}-data-store"

However, I would not want dash-extensions to store the key in local storage as it now has the session id:

Key                   Value
data-store	"SERVERSIDE_{"backend_uid": "RedisBackend", "key": "87sdghfoib3k083ip"}"

Should be:

Key                   Value
data-store	"SERVERSIDE_{"backend_uid": "RedisBackend"}"

Retrieving would be some kind of decorator that calls the session id function and appends it to get the key it needs to retrieve from.

@Lxstr
Copy link
Author

Lxstr commented Jul 11, 2024

It looks like the use of _get_cache_id and _get_session_id is no longer included since 1.0.

Now Serverside is a type initialised during each callback.

class Serverside(Generic[T]):

    def __init__(self, value: T, key: str = None, backend: Union[ServersideBackend, str, None] = None):
        self.value = value
        self.key: str = str(uuid.uuid4()) if key is None else key
        self.backend_uid: str = backend.uid if isinstance(backend, ServersideBackend) else backend

If you do not set a key, it will automatically generate on each callback run, resulting in multiple cache entries.

My workaround for sessions:

return Serverside(df, key=f"data-store-{get_data_cache_key()}"),
def get_data_cache_key():
    if session.get("data_cache_key") is None:
        session["data_cache_key"] = str(secrets.token_hex(16))
    return session.get("data_cache_key")

However, this does place a cache key for potentially sensitive data in local storage, which is less than perfect, so I've started a PR at #340

@Lxstr Lxstr closed this as completed Jul 11, 2024
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

No branches or pull requests

3 participants