MediaFile Grace Period using KEEP_DELAY = 5 seconds#1494
MediaFile Grace Period using KEEP_DELAY = 5 seconds#1494nthmost merged 13 commits intostreamlit:developfrom
Conversation
tvst
left a comment
tvst
left a comment
There was a problem hiding this comment.
Looks good! But PTAL at potential improvements in the comments
lib/streamlit/MediaFileManager.py
Outdated
| LOGGER.debug( | ||
| "Sessions still active: %r", self._files_by_session_and_coord.keys() | ||
| ) | ||
| self._del_expired_files() |
There was a problem hiding this comment.
This means that files from one session will linger until some other session happens to start or end, and that even happens to take place KEEP_DELAY secs later. To improve this, how about running _del_expired_files from a timer?
The only wrinkle with this idea is that one of the places where clear_session_files is called is when the script is about to run, so we can't simply add a time.sleep(KEEP_DELAY) here. So we have to schedule the run via Tornado's IOLoop instead. But since this function is always called from the actual Script thread, we can't just use tornado.ioloop.IOLoop.current() to get the current IOLoop. So we need to do a tiny bit of piping:
# In Server.py __init__ method
media_file_manager.set_ioloop(ioloop = self._ioloop)
# In MediaFileManager
def __init__(self):
# (..._)
self._ioloop = None
def set_ioloop(self, ioloop):
self._ioloop = ioloop
def clear_session_files(...):
# (...)
self._ioloop.call_later(KEEP_DELAY_SEC, self._del_expired_files)There was a problem hiding this comment.
Yeah this seems to work. I've had to adjust KEEP_DELAY_SEC to be longer (4 or 5, more testing required).
A lot of tests need to be adjusted now, due to the change in Server.py...
nthmost
changed the title
|
Thanks for the fix! Looks like the issue is gone in the nightly version 🎉 |
* develop: Formatting changes from `make pyformat` (#1621) Add config to disable CSRF + conflict check with CORS (#1571) MediaFile Grace Period using KEEP_DELAY = 5 seconds (#1494) Fix server.cookieSecret config option. (#1603) Create a config option to toggle websocket compression (#1544) Remove Python 2 and Python 3.5 staleness (#1600) Disable/Remove scroll when in full screen mode (#1586) Better welcome message when running `streamlit hello` (#1585) Scale image to approximate size on screen (#1594)
|
Hi, I'm currently having this issue with an AppEngine hosted flask app which generates multiple plots on the fly. The app takes a user loaded file, processes it, then charts the resultant analyses - however, the charts randomly load,,, sometimes the user will get 2 of 5, other times 3 of 5, and which charts load is random. Any thought? |
4 participants
Fixes #1440, Fixes #1294, Fixes #1454
Description:
The MediaFileManager currently uses a method of management that involves assuming that when a Session has completed its run, those files are no longer needed and can be safely deleted.
However, the reality is that the browser may not have had enough time between session runs to request that file, particularly in rapid-fire script reruns, resulting in broken images being shown to the user as the browser tries to request files that have already been deleted.
Examples of rapid-fire session creation-and-destruction include:
st.imageelement to show each frame from that video.This PR introduces a KEEP_DELAY (which could potentially be configurable, but for now is statically defined) in
MediaFileManager.py(MFM) and presently set to 5 seconds (a setting found by experimentation... I tried 1-2 seconds and that didn't work too well).When a MediaFile is created, it gets a
ttd(time to die) time set totime.time() + KEEP_DELAYEach time a session finishes, it triggers the checking of each file_id currently stored in the MFM to see if its
ttdhas expired compared totime.time()This results in enough of a grace period before wiping expired media files to eliminate the effects seen in #1440 and similar bugs (see issues listed above).
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.