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.

Sign up for GitHub

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

Jump to bottom

Let's be smarter with caching #102

Closed
slifty opened this issue Mar 3, 2023 · 1 comment
Closed

Let's be smarter with caching #102

slifty opened this issue Mar 3, 2023 · 1 comment

Comments

@slifty
Copy link
Contributor

slifty commented Mar 3, 2023

Caching is complicated!

I implemented a filesystem cache to help address the fact that we have to crawl every step of a given path for any given SFTP call. This is fine, but the resulting implementation is a little too aggressive.

I'd like to propose the following change to the cache:

  1. The cache is never used for direct queries -- that is, if opening a directory the cache will always be ignored for the final directory call.

  2. Any time a cached folder claims to not contain a given item, we try re-loading that cached folder.

There may be some other ideas as well, but this should help fix a large number of our current cache-related bugs.
slifty added a commit that referenced this issue Mar 7, 2023
@slifty
Add cache bypasses
c170511 
There are a few places where our caching logic was making it likely for
the cache to result in false misses.  The intent of the cache is pretty
specific: to avoid making API calls when crawling up the tree.

These changes really restrict the cache to those pieces, and should
prevent use of the cache for any item that is being directly accessed by
an SFTP call.  It also adds logic to refresh the cache one time whenever
there is a detected "miss" when retrieving a folder.

Issue #102
slifty added a commit that referenced this issue Mar 7, 2023
@slifty
Add cache bypasses
331f753 
There are a few places where our caching logic was making it likely for
the cache to result in false misses.  The intent of the cache is pretty
specific: to avoid making API calls when crawling up the tree.

These changes really restrict the cache to those pieces, and should
prevent use of the cache for any item that is being directly accessed by
an SFTP call.  It also adds logic to refresh the cache one time whenever
there is a detected "miss" when retrieving a folder.

Issue #102
@slifty slifty mentioned this issue Mar 7, 2023
Merged
slifty added a commit that referenced this issue Mar 7, 2023
@slifty
Add cache bypasses
9fff82d 
There are a few places where our caching logic was making it likely for
the cache to result in false misses.  The intent of the cache is pretty
specific: to avoid making API calls when crawling up the tree.

These changes really restrict the cache to those pieces, and should
prevent use of the cache for any item that is being directly accessed by
an SFTP call.  It also adds logic to refresh the cache one time whenever
there is a detected "miss" when retrieving a folder.

Issue #102
@slifty
Copy link
Contributor Author

slifty commented Mar 7, 2023

I think caching is now about as smart as I was intending when I opened this issue... so I'll close it, knowing this probably won't be the last cache related issue...

@slifty slifty closed this as completed Mar 7, 2023
@slifty slifty mentioned this issue Mar 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@slifty