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

Add a persistent image cache #291

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

CosmicHorrorDev
Copy link
Collaborator

This will eventually resolve #234

@CosmicHorrorDev CosmicHorrorDev force-pushed the image-cache branch 3 times, most recently from 851f7ba to ce16fea Compare April 7, 2024 20:26
@CosmicHorrorDev
Copy link
Collaborator Author

Just some random tidbits:

  • Useful looking crate for handling HTTP cache semantics (http-cache-semantics). Potentially this crate as a higher level one depending on how much async is a requirement there (http-cache)
  • There's some interop required to get the right info out of ureq but that's not the worst
  • The DB format is a bit more static with redb than it would be with some SQL DB that can be changed with migrations. I think we can have some consistent header with the internal version and TTL so that we can mix-n-match versions a bit as long as the table layout doesn't change

@CosmicHorrorDev CosmicHorrorDev added C-enhancement Category: New feature or request A-image Area: Image decoding/rendering/etc. labels Apr 21, 2024
@CosmicHorrorDev
Copy link
Collaborator Author

thread 'image::cache::tests::layers' panicked at src/image/cache/tests.rs:306:66:
called `Result::unwrap()` on an `Err` value: Failed creating database

Caused by:
    Database already open. Cannot acquire lock.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    image::cache::tests::layers

Guess who has two thumbs and gets to port things away from redb? 👍😞👍

@CosmicHorrorDev
Copy link
Collaborator Author

Still a little ways to go, but getting closer at least 🎉

running 1 test
test image::cache::tests::layers ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 52 filtered out; finished in 0.03s

@CosmicHorrorDev CosmicHorrorDev force-pushed the image-cache branch 2 times, most recently from 55ec404 to 73f5533 Compare June 24, 2024 10:04
@CosmicHorrorDev CosmicHorrorDev force-pushed the image-cache branch 5 times, most recently from 6fb7c8e to 8b2ebb9 Compare November 4, 2024 04:28
rebase this

test(image_cache): Add tests for image cache headers

rebase this

f

Migrate over to `http-cache-semantics`

f

refactor(tests): Drop wiremock (Inlyne-Project#320)

* docs: Describe what we use deps for

* refactor(tests): Switch `wiremock` for a custom impl

refactor(tests): simplify the file server (Inlyne-Project#321)

Refactor local image caching

Hashing out more of the impl

needs _soooo_ much refactoring

Dirty rebase artifacts. Fix later

rebase me away

slowly getting the final pieces together

time to update with `main`

Nail down most of the routing

Dont pre-render SVGs before storage

Get more of the custom test env setup

Get the sanity test working

refactor(cache): Switch db from redb to sqlite

Use our shiny new test utils

refactor: Delete dead test code

rebase into commit that added it

rename images.data -> images.image

remove redb dep

lalalalala

f

get local image fetching working

Get more tests sorted out

Shrank test data image rebase original file out of git history

Test suite is getting better

getting closer

Add etag flow to test server

test: Add support for dynamic image test servers

rebase this: lru test mostly working

rebase this

some cleanup

more image cache tests
@CosmicHorrorDev
Copy link
Collaborator Author

CosmicHorrorDev commented Nov 11, 2024

The test suite is a good representation of the current state of the PR

$ cargo t image::cache
...

running 17 tests
test image::cache::tests::global_ttl ... ignored, TODO: waiting for garbage collection
test image::cache::tests::lru ... ignored, TODO: waiting for garbage collection
test image::cache::tests::mutli_client_mash ... ignored, TODO: waiting for garbage collection
test image::cache::tests::remote_404_error ... ok
test image::cache::tests::private_cache ... ok
test image::cache::tests::etag_refresh_same ... ok
test image::cache::tests::selectively_stores ... ok
test image::cache::tests::remote_layers ... ok
test image::cache::tests::corrupt_db_entry ... ok
test image::cache::tests::etag_refresh_different ... ok
test image::cache::tests::local_invalidation ... ok
test image::cache::tests::invalid_img ... ok
test image::cache::tests::stats ... ok
test image::cache::tests::local_svg_layers ... ok
test image::cache::tests::local_layers ... ok
test image::cache::tests::past_max_age_refetch ... ok
test image::cache::tests::remote_svg_layers ... ok

test result: ok. 14 passed; 0 failed; 3 ignored; 0 measured; 54 filtered out; finished in 0.27s

The only thing left to implement is garbage collection (including probably updating entry's m-times) and wiring up the cache to actually be used by the application

Potential future work

  • Integrating a lot more metrics
    • Counters for image loads (from source / l1 cached / l2 cached / errors)
    • Timings for image loads
  • Switching DB connections to use a pool to manage re-using connections
  • Make the stats output a lot prettier
  • Switch the generation to a content hash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-image Area: Image decoding/rendering/etc. C-enhancement Category: New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache downloaded images
1 participant