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

DASHBOARD_CACHE feature not clearing the cache as expected #15518

Closed
3 tasks done
john-bodley opened this issue Jul 1, 2021 · 3 comments
Closed
3 tasks done

DASHBOARD_CACHE feature not clearing the cache as expected #15518

john-bodley opened this issue Jul 1, 2021 · 3 comments
Assignees
Labels
#bug Bug report

Comments

@john-bodley
Copy link
Member

john-bodley commented Jul 1, 2021

I believe there are a number of issues related to the DASHBOARD_CACHE feature which is currently defined as being in development. These issues are evident when editing dashboard metadata, i.e., adding an owner, and the upon re-editing the previous edits are not shown given that the cached response is leveraged.

Specifically:

  1. The check_modified and not object_session(obj).is_modified(obj) check evaluates to True when updating the dashboard meaning the cache is not cleared.
  2. The clear_cache method (or similar) does not clear the etag caches associated with the DashboardRestApi responses defined here.
  3. The etag_cache logic may not be correct regarding exposing how the cache key is made and thus when one tries to delete the memoized object, i.e., cache_manager.cache.delete_memoized(DashboardRestApi.get, self.id) (or similar) the cache key is different. Note Flask-Caching memoization is complex and I think we should try to lean heavily on their own memoize function and merely wrap it in a similar vein to Memoization with Locking  #15396.

Note I'm not sure whether the upside of caching metastore queries (which needs a very robust cache eviction policy as one needs to read their own writes) is warranted. It's also unclear whether the ETag cache logic can/should be decoupled from the DASHBOARD_CACHE feature.

Environment

(please complete the following information):

  • superset version: superset version
  • python version: python --version
  • node.js version: node -v

Checklist

Make sure to follow these steps before submitting your issue - thank you!

  • I have checked the superset logs for python stacktraces and included it here as text if there are any.
  • I have reproduced the issue with at least the latest released version of superset.
  • I have checked the issue tracker for the same issue and I haven't found one similar.

Additional context

Add any other context about the problem here.

@john-bodley john-bodley added the #bug Bug report label Jul 1, 2021
@ktmud
Copy link
Member

ktmud commented Jul 12, 2021

To keep things simple, I think we should get rid of etag_cache and utilize only internal authentication-free cache at the Dashboard resource level. The API can have special logics to read from a cached dict instead of Dashboard entity.

@stale
Copy link

stale bot commented Apr 30, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

@stale stale bot added the inactive Inactive for >= 30 days label Apr 30, 2022
@john-bodley
Copy link
Member Author

This is now obsolete since the DASHBOARD_CACHE feature flag was removed by @michael-s-molina in #26349.

@stale stale bot removed the inactive Inactive for >= 30 days label Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
#bug Bug report
Projects
None yet
Development

No branches or pull requests

4 participants