Skip to content

Commit

Permalink
--zarrman-cache-bytes
Browse files Browse the repository at this point in the history
  • Loading branch information
jwodder committed Aug 8, 2024
1 parent 8336a6b commit 58eeb5e
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 28 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ In Development
- Added doc comments to much of the code
- Return 502 status when a backend returns an invalid response
- Require `--api-url` (and other URLs retrieved from APIs) to be HTTP(S)
- Format all log lines as JSON
- Add logging of Zarr manifest cache events
- Limit Zarr manifest cache by total size of entries
- Add a `-Z`/`--zarrman-cache-bytes` option for setting the cache size
- Expire idle Zarr manifest cache entries
- Log Zarr manifest cache entries every hour

v0.4.0 (2024-07-09)
-------------------
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,7 @@ Options
- `-T <TITLE>`, `--title <TITLE>` — Specify the site name to use in HTML/web
views of collections (used inside `<title>`'s and as the root breadcrumb
text) [default: dandidav]

- `-Z <INT>`, `--zarrman-cache-bytes <INT>` — Specify the maximum number of
bytes of parsed Zarr manifest files to store in the Zarr manifest cache at
once [default: 100 MiB]
4 changes: 4 additions & 0 deletions src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ pub(crate) static FAST_NOT_EXIST: &[&str] = &[".bzr", ".git", ".nols", ".svn"];
/// Interval between periodic logging of the Zarr manifest cache's contents
pub(crate) const ZARR_MANIFEST_CACHE_DUMP_PERIOD: Duration = Duration::from_secs(3600);

/// Default size of the Zarr manifest cache; the cache is limited to storing no
/// more than this many bytes of parsed manifests at once
pub(crate) const ZARR_MANIFEST_CACHE_TOTAL_BYTES: u64 = 100 * 1024 * 1024; // 100 MiB

#[cfg(test)]
mod tests {
use super::*;
Expand Down
11 changes: 7 additions & 4 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ mod paths;
mod s3;
mod streamutil;
mod zarrman;
use crate::consts::{
CSS_CONTENT_TYPE, DEFAULT_API_URL, SERVER_VALUE, ZARR_MANIFEST_CACHE_DUMP_PERIOD,
};
use crate::consts::*;
use crate::dandi::DandiClient;
use crate::dav::{DandiDav, Templater};
use crate::httputil::HttpUrl;
Expand Down Expand Up @@ -67,6 +65,11 @@ struct Arguments {
/// Site name to use in HTML collection pages
#[arg(short = 'T', long, default_value = env!("CARGO_PKG_NAME"))]
title: String,

/// Limit the Zarr manifest cache to storing no more than this many bytes
/// of parsed manifests at once
#[arg(short = 'Z', long, default_value_t = ZARR_MANIFEST_CACHE_TOTAL_BYTES, value_name = "INT")]
zarrman_cache_bytes: u64,

Check warning on line 72 in src/main.rs

View check run for this annotation

Codecov / codecov/patch

src/main.rs#L71-L72

Added lines #L71 - L72 were not covered by tests
}

// See
Expand Down Expand Up @@ -99,7 +102,7 @@ fn main() -> anyhow::Result<()> {
async fn run() -> anyhow::Result<()> {
let args = Arguments::parse();
let dandi = DandiClient::new(args.api_url)?;
let zarrfetcher = ManifestFetcher::new()?;
let zarrfetcher = ManifestFetcher::new(args.zarrman_cache_bytes)?;
zarrfetcher.install_periodic_dump(ZARR_MANIFEST_CACHE_DUMP_PERIOD);
let zarrman = ZarrManClient::new(zarrfetcher);

Check warning on line 107 in src/main.rs

View check run for this annotation

Codecov / codecov/patch

src/main.rs#L105-L107

Added lines #L105 - L107 were not covered by tests
let templater = Templater::new(args.title)?;
Expand Down
4 changes: 0 additions & 4 deletions src/zarrman/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,5 @@ pub(super) static MANIFEST_ROOT_URL: &str =
/// `{ENTRY_DOWNLOAD_PREFIX}/{zarr_id}/{entry_path}`.
pub(super) static ENTRY_DOWNLOAD_PREFIX: &str = "https://dandiarchive.s3.amazonaws.com/zarr/";

/// Limit the manifest cache to storing no more than this many bytes of parsed
/// manifests at once
pub(super) const MANIFEST_CACHE_TOTAL_BYTES: u64 = 100 * 1024 * 1024; // 100 MiB

/// Expire any manifest cache entries that haven't been accessed for this long
pub(super) const MANIFEST_CACHE_IDLE_EXPIRY: Duration = Duration::from_secs(300);
39 changes: 19 additions & 20 deletions src/zarrman/fetcher.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::consts::{MANIFEST_CACHE_IDLE_EXPIRY, MANIFEST_CACHE_TOTAL_BYTES, MANIFEST_ROOT_URL};
use super::consts::{MANIFEST_CACHE_IDLE_EXPIRY, MANIFEST_ROOT_URL};
use super::manifest::Manifest;
use super::resources::ManifestPath;
use super::util::{Index, ZarrManError};
Expand Down Expand Up @@ -33,26 +33,25 @@ impl ManifestFetcher {
/// # Errors
///
/// Returns an error if construction of the inner `reqwest::Client` fails
pub(crate) fn new() -> Result<Self, BuildClientError> {
pub(crate) fn new(cache_size: u64) -> Result<Self, BuildClientError> {
let inner = Client::new()?;
let cache: Cache<ManifestPath, Arc<Manifest>> =
CacheBuilder::new(MANIFEST_CACHE_TOTAL_BYTES)
.name("zarr-manifests")
.weigher(|_, manifest: &Arc<Manifest>| {
u32::try_from(manifest.get_size()).unwrap_or(u32::MAX)
})
.time_to_idle(MANIFEST_CACHE_IDLE_EXPIRY)
.eviction_listener(|path, manifest, cause| {
tracing::debug!(
cache_event = "evict",
cache = "zarr-manifests",
manifest = %path,
manifest_size = manifest.get_size(),
?cause,
"Zarr manifest evicted from cache",
);
})
.build();
let cache: Cache<ManifestPath, Arc<Manifest>> = CacheBuilder::new(cache_size)
.name("zarr-manifests")
.weigher(|_, manifest: &Arc<Manifest>| {
u32::try_from(manifest.get_size()).unwrap_or(u32::MAX)
})
.time_to_idle(MANIFEST_CACHE_IDLE_EXPIRY)
.eviction_listener(|path, manifest, cause| {
tracing::debug!(

Check warning on line 45 in src/zarrman/fetcher.rs

View check run for this annotation

Codecov / codecov/patch

src/zarrman/fetcher.rs#L36-L45

Added lines #L36 - L45 were not covered by tests
cache_event = "evict",
cache = "zarr-manifests",
manifest = %path,
manifest_size = manifest.get_size(),
?cause,
"Zarr manifest evicted from cache",

Check warning on line 51 in src/zarrman/fetcher.rs

View check run for this annotation

Codecov / codecov/patch

src/zarrman/fetcher.rs#L49-L51

Added lines #L49 - L51 were not covered by tests
);
})
.build();
let manifest_root_url = MANIFEST_ROOT_URL
.parse::<HttpUrl>()
.expect("MANIFEST_ROOT_URL should be a valid HTTP URL");
Expand Down

0 comments on commit 58eeb5e

Please sign in to comment.