-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
fix: Remove double pickling for cached payloads #10222
Conversation
56565d8
to
0d0b4ff
Compare
0d0b4ff
to
cf613d4
Compare
cf613d4
to
78d9f7b
Compare
@@ -260,14 +258,8 @@ def get_df_payload( # pylint: disable=too-many-locals,too-many-statements | |||
if is_loaded and cache_key and cache and status != utils.QueryStatus.FAILED: | |||
try: | |||
cache_value = dict(dttm=cached_dttm, df=df, query=query) | |||
cache_binary = pkl.dumps(cache_value, protocol=pkl.HIGHEST_PROTOCOL) | |||
|
|||
logger.info( |
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.
I annexed this as we no longer a priori know the size of the payload and it seems somewhat like an unnecessary comment.
stats_logger.incr("set_cache_key") | ||
cache.set(cache_key, cache_binary, timeout=self.cache_timeout) | ||
cache.set(cache_key, cache_value, timeout=self.cache_timeout) |
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.
random thought [out of scope for this PR], we should time this operation eventually
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.
Co-authored-by: John Bodley <[email protected]>
SUMMARY
It seems we pickle the visualization payloads et al. prior to caching via the Flask-Caching package, however for all backends as part of the set/get logic the object to cache is pickled and the unpicked respectively, i.e., for Redis the set and get methods call
dump_object
andload_object
respectively which in turn callspickle.dumps
andpickle.loads
, hence we were actually double pickling the payloads.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
CI.
ADDITIONAL INFORMATION