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

Script caching isn't working properly #990

Open
ctrlaltdavid opened this issue Jan 30, 2021 · 9 comments · May be fixed by #1012
Open

Script caching isn't working properly #990

ctrlaltdavid opened this issue Jan 30, 2021 · 9 comments · May be fixed by #1012
Labels
bug Something isn't working confirmed The bug has been successfully reproduced. Severity: Medium Important functionality is affected, but a workaround exists stale Issue / PR has not had activity

Comments

@ctrlaltdavid
Copy link
Collaborator

Checking whether script file has changed and needs to be redownloaded (instead of just using whatever's in cache).
Respecting script file expiry dates (i.e., redownload if expired).
Cache-busting.

@ctrlaltdavid ctrlaltdavid added the bug Something isn't working label Jan 30, 2021
@digisomni
Copy link
Member

This happens with models too! Generally requests and caching aren't really checked I think according to the HTTP spec.

@HifiExperiments
Copy link
Collaborator

I’ve looked into this before and I think the cachebusting etc is necessary because we force our HTTP requests to use qnetworkrequest::PreferCache in https://github.com/vircadia/vircadia/blob/ec1c8f2b87187c5be8f5d208b539b6848ebd97b4/libraries/networking/src/HTTPResourceRequest.cpp

the default value, PreferNetwork, might just solve a lot of these problems: https://doc.qt.io/qt-5/qnetworkrequest.html#CacheLoadControl-enum

@HifiExperiments
Copy link
Collaborator

#92

@daleglass
Copy link
Contributor

Dev meeting: Need to check what exactly does PreferNetwork do: does it go to the server every time to check the timestamp?

@daleglass
Copy link
Contributor

Also, do we store expiration times in our cache?

@daleglass daleglass added confirmed The bug has been successfully reproduced. Severity: Medium Important functionality is affected, but a workaround exists labels Feb 6, 2021
@HifiExperiments
Copy link
Collaborator

it sounds like even PreferCache respects expiry times, but I guess the only way to find out is to try it

I would assume hifi originally chose PreferCache because it means fewer network requests to check timestamps, but the trade off seems like it’s not worth it. or perhaps if the extra requests are too much, we have a “developer mode” that lets you temporarily switch to PreferNetwork while testing models/scripts, but most people still use PreferCache. there’s already a switch in the code, but the alternative is AlwaysNetwork, which doesn’t seem like what we want either

@HifiExperiments HifiExperiments linked a pull request Feb 9, 2021 that will close this issue
@HifiExperiments HifiExperiments linked a pull request Feb 9, 2021 that will close this issue
@stale
Copy link

stale bot commented Jun 6, 2021

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.

@stale stale bot added the stale Issue / PR has not had activity label Jun 6, 2021
@daleglass daleglass removed the stale Issue / PR has not had activity label Jul 17, 2021
@stale
Copy link

stale bot commented Jan 13, 2022

Hello! Is this still an issue?

@stale stale bot added the stale Issue / PR has not had activity label Jan 13, 2022
@HifiExperiments HifiExperiments removed the stale Issue / PR has not had activity label Jan 14, 2022
@stale
Copy link

stale bot commented Jul 13, 2022

Hello! Is this still an issue?

@stale stale bot added the stale Issue / PR has not had activity label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed The bug has been successfully reproduced. Severity: Medium Important functionality is affected, but a workaround exists stale Issue / PR has not had activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants