Troubles with caching #58

Closed
Morpho opened this Issue Mar 9, 2012 · 10 comments

Projects

None yet

3 participants

@Morpho
Morpho commented Mar 9, 2012

Hey,
I have an application that sends email notification to users, when a new entry is made to there "wall" inside an app. Now that you only cache user's email for one day, I dont have the emails addresses anymore, after a user doesnt log in for 2 days and the token expires. I dont think its a good idea to rely on the fact that all users will always login to have their data present. When there is a user not logging in for some days, I wont have a valid accesstoken and so I wont have his locale, his email, etc.

The caching system should just overwrite! its cached values every day, but the key itself should not expire, so at least an old email or locale will always be present.

What do you think?

@mbaechtold

There was an extended permission called "offline_access", but Facebook has deprecated it:
http://developers.facebook.com/docs/offline-access-deprecation/

Instead, Facebook offers a way to extend a user's access token up to 60 days.
@jgorset : What do you think, would that be feasible with Fandjango?

And just another idea: you could store a user's attributes in a new database (some kind of a "profile app") but I don't know if this is against the policies of Facebook (see chapter "II. Storing and Using Data You Receive From Us" of the Facebook Platform Policies)

@jgorset
Owner
jgorset commented Mar 11, 2012

I think you make a good case for persisting the cache for more than 24 hours. I've increased the duration that a user's attributes are cached to 30 days, but be sure to populate the cache while you still have an active access token!

$ pip install git://github.com/jgorset/fandjango.git@d5e64af2132b4e63521ef19e8d0c273f8a8a09b1

@mbaechtold is right on the money with regards to extended access tokens — in fact, fandjango has been extending access tokens since 4.0.3.

Ya'll deserve some 🍰.

@mbaechtold

I should have read the release notes before posting :-)

@Morpho
Morpho commented Mar 12, 2012

Thanks @jgorset and @mbaechtold for the quick reply. Increasing the duration is what I also did in my copy of fandjango as a quick fix. I'm just not sure if that fixes the actual problem. In my case, now if a user doesnt use the app for more than 30 days, I wont be able to notify him via email anymore because his email address wont be in the cache anymore. Besides of that, now if a user changes his email or locale it will take up to 30 days until the app knows about that new email or locale, because it will only check for new values when the cache expires.

What do you think about a different logic inside the cached_property function, that uses the time argument as an update frequency instead of an expiry time? So the cache would get updated every 2 days or so, but an old value would always be present.

Something like that:


def cached_property(**kwargs):
    """Cache the return value of a property."""
    def decorator(function):
        @wraps(function)
        def wrapper(self):
            key = 'fandjango.%(model)s.%(property)s_%(pk)s' % {
                'model': self.__class__.__name__,
                'pk': self.pk,
                'property': function.__name__
            }

            cached_value = cache.get(key)
            already_cached =  cache.get('%s_already' % key)

            delta = timedelta(**kwargs)

            if already_cached is None: # if it's None then we have to update!
                try:
                        value = function(self)
                        cache.set(key, value, 60*60*24*100) # keep it 100 days in cache
                except Exception: # maybe a more define exception would be better
                    pass
                cache.set('%s_already' % key, True, delta.days * 86400 + delta.seconds) # set it even if update failed
            else:
                value = cached_value

            return value
        return wrapper
    return decorator

This way the function would check every X days for updates and store it in a cache key with a static expiry time of 100 days. What do you think about that? Or maybe I really thought wrong and this is stupid?

@mbaechtold

Besides of that, now if a user changes his email or locale it will take up to 30 days until the app knows about that new email or locale, because it will only check for new values when the cache expires.

What about the Real-time Updates of the Graph API: https://developers.facebook.com/docs/reference/api/realtime/

I've not used this yet...

@jgorset
Owner
jgorset commented Mar 13, 2012

I think this is a great use case for real-time updates, and I would love for Fandjango to support it. Ideally, the endpoint for real-time updates should populate the cache (which, in turn, should probably live for more than 30 days).

@Morpho
Morpho commented Mar 13, 2012

Of course that would be the most elegant solution!
I could imagine a new setting like FACEBOOK_APPLICATION_SUBSCRIBE_TO = ['email', 'locale', 'location']
And two management commands like: ./manage.py fandjango subscribe | ./manage.py fandjango unsubscribe
Next week I will try to code it as my time permits

@Morpho
Morpho commented Apr 1, 2012

Ok, I looked into the Facebook Realtime API. It's actually not that perfect as I thought before. When there is a change in the user's data, the API pushes a POST request telling the app that there is a change. But the new value that actually changed is never sent by facebook, so we still have to have a valid access token and query the graph api to get the new values after a change. In my opinion the realtime API is pretty useless like that, because its better and easier to update user's details every 2 days or so when the user is logging in to the app. Of course this should be an asynchronous task, for example using "django celery". I think that is the best solution. What do you think?

Besides of that, I personally don't think its a good idea to store userdata in a cache-key that has an expiry, because so I can never be sure that a value actually exists. I prefer having at least an old value over having no value at all of a specific user field after the user did not log in for 30 days.

For example a common task is to send an email to every app user that didnt use the app for 1 month or more. With the current solution we couldn't do it, because after 30 days we wont have their emails anymore. Anyway, the design decision is up to you @jgorset, I hope I dont annoy you with all the stuff I write here ;-) I might just create my own fork of fandjango.

@jgorset
Owner
jgorset commented Apr 3, 2012

Besides of that, I personally don't think its a good idea to store userdata in a cache-key that has an expiry, because so I can never be sure that a value actually exists. I prefer having at least an old value over having no value at all of a specific user field after the user did not log in for 30 days.

For example a common task is to send an email to every app user that didnt use the app for 1 month or more. With the current solution we couldn't do it, because after 30 days we wont have their emails anymore

Alright, I'm convinced — we will persist user attributes in the database instead of caching them. I hope to get around to it sometime this month, but please feel free to submit a pull request if you have the opportunity.

@jgorset
Owner
jgorset commented May 4, 2013

Closed by #89.

@jgorset jgorset closed this May 4, 2013
@jgorset jgorset referenced this issue May 4, 2013
Merged

Persistence #89

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment