Merge branch 'android-collections'
Build and test / Desktop (Linux) (push) Failing after 18m53s
Build and test / Layer separation (push) Successful in 41s
Traceability / Requirement traces (push) Failing after 1m4s
🐳 Android image / Build and push (push) Successful in 6s
Build and test / android-image (push) Successful in 5s
Build and test / Android (aarch64) (push) Failing after 9m40s
Build and test / Desktop (Linux) (push) Failing after 18m53s
Build and test / Layer separation (push) Successful in 41s
Traceability / Requirement traces (push) Failing after 1m4s
🐳 Android image / Build and push (push) Successful in 6s
Build and test / android-image (push) Successful in 5s
Build and test / Android (aarch64) (push) Failing after 9m40s
Touch multi-selection in the grid, filing a selection into a collection without a drag, and taking a collection offline from a held row.
This commit is contained in:
@@ -41,7 +41,7 @@
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use dr_types::{ImageId, Tier};
|
||||
use rusqlite::Connection;
|
||||
use rusqlite::{Connection, OptionalExtension as _};
|
||||
|
||||
use crate::error::CatalogError;
|
||||
|
||||
@@ -299,6 +299,71 @@ impl Cache {
|
||||
self.set_pinned(conn, images, false)
|
||||
}
|
||||
|
||||
/// Release the pin *and* delete the bytes it was holding.
|
||||
///
|
||||
/// The destructive half of the pair [`unpin`](Self::unpin) deliberately is
|
||||
/// not. Unpinning answers "stop promising"; this answers "give me the disk
|
||||
/// back", which is the question actually being asked when a trip is over
|
||||
/// and the device is full. Leaving those gigabytes to sit until some future
|
||||
/// eviction happens to want the room is not an answer to it.
|
||||
///
|
||||
/// Nothing is lost that cannot be fetched again: the original lives on the
|
||||
/// server, and the catalog row, the ratings and the edit graph are all
|
||||
/// untouched here — they are authoritative and small (FR-NC-6b).
|
||||
///
|
||||
/// Returns how many images were released and how many bytes that freed.
|
||||
/// A file that has already vanished frees nothing and is still counted as
|
||||
/// released, because the row describing it goes either way.
|
||||
pub fn release(
|
||||
&self,
|
||||
conn: &Connection,
|
||||
images: &[ImageId],
|
||||
) -> Result<(usize, u64), CatalogError> {
|
||||
if images.is_empty() {
|
||||
return Ok((0, 0));
|
||||
}
|
||||
|
||||
// Read the paths before the rows are rewritten: `forget` clears `path`,
|
||||
// and a file whose name has been forgotten cannot be deleted.
|
||||
let mut held = Vec::new();
|
||||
{
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT bytes, path FROM image_cache
|
||||
WHERE image_id = ?1 AND path IS NOT NULL",
|
||||
)?;
|
||||
for image in images {
|
||||
if let Some(row) = stmt
|
||||
.query_row(rusqlite::params![image.0 as i64], |r| {
|
||||
Ok((r.get::<_, i64>(0)? as u64, r.get::<_, String>(1)?))
|
||||
})
|
||||
.optional()?
|
||||
{
|
||||
held.push(row);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let mut freed = 0u64;
|
||||
for (bytes, rel) in &held {
|
||||
let abs = self.dir.join(rel);
|
||||
match std::fs::remove_file(&abs) {
|
||||
Ok(()) => freed += bytes,
|
||||
// Already gone is the ordinary case after a crash mid-write,
|
||||
// not a failure: the row still has to go, or the cache accounts
|
||||
// for space nothing occupies.
|
||||
Err(e) => log::debug!("releasing {}: {e}", abs.display()),
|
||||
}
|
||||
}
|
||||
|
||||
// Unpin first, then forget. The other order would leave a pinned row
|
||||
// claiming an original it no longer has, which `pending_pins` would
|
||||
// then dutifully download again — the exact opposite of what was asked.
|
||||
self.set_pinned(conn, images, false)?;
|
||||
self.forget(conn, images)?;
|
||||
|
||||
Ok((images.len(), freed))
|
||||
}
|
||||
|
||||
fn set_pinned(
|
||||
&self,
|
||||
conn: &Connection,
|
||||
@@ -609,6 +674,66 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn releasing_a_pin_frees_the_disk_it_was_holding() {
|
||||
// What "remove the local copies" has to mean. Unpinning alone leaves
|
||||
// the bytes for a future eviction to notice, which is no answer at all
|
||||
// to a device that is full now.
|
||||
let (cat, cache, dir, ids) = fixture(2);
|
||||
let bytes = vec![0u8; 700];
|
||||
cache
|
||||
.store(cat.connection(), ids[0], "a.CR2", &bytes, true, 10)
|
||||
.unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[1], "b.CR2", &bytes, true, 20)
|
||||
.unwrap();
|
||||
|
||||
let (released, freed) = cache.release(cat.connection(), &ids).unwrap();
|
||||
assert_eq!(released, 2);
|
||||
assert_eq!(freed, 1400);
|
||||
|
||||
assert!(!cache.holds_original(cat.connection(), ids[0]));
|
||||
assert_eq!(cache.usage(cat.connection()).unwrap().pinned_bytes, 0);
|
||||
|
||||
// The files themselves, not just the bookkeeping: a row cleared over a
|
||||
// file still on disk is how a cache comes to hold gigabytes it does not
|
||||
// know about.
|
||||
let left: Vec<_> = walk_files(&dir);
|
||||
assert!(left.is_empty(), "files remain on disk: {left:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_released_pin_is_not_downloaded_all_over_again() {
|
||||
// The failure mode of releasing in the wrong order: bytes deleted while
|
||||
// the row still says an original is wanted, so the next pin fetch pulls
|
||||
// the whole trip back down.
|
||||
let (cat, cache, _dir, ids) = fixture(1);
|
||||
cache
|
||||
.store(cat.connection(), ids[0], "a.CR2", &vec![0u8; 100], true, 10)
|
||||
.unwrap();
|
||||
|
||||
cache.release(cat.connection(), &ids).unwrap();
|
||||
|
||||
assert!(cache.pending_pins(cat.connection()).unwrap().is_empty());
|
||||
}
|
||||
|
||||
/// Every file under `dir`, for asserting that a release left nothing.
|
||||
fn walk_files(dir: &Path) -> Vec<PathBuf> {
|
||||
let mut out = Vec::new();
|
||||
let Ok(entries) = std::fs::read_dir(dir) else {
|
||||
return out;
|
||||
};
|
||||
for entry in entries.flatten() {
|
||||
let path = entry.path();
|
||||
if path.is_dir() {
|
||||
out.extend(walk_files(&path));
|
||||
} else {
|
||||
out.push(path);
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pinned_bytes_do_not_count_against_the_budget() {
|
||||
// Otherwise a large pin starves the passive cache into evicting
|
||||
|
||||
Reference in New Issue
Block a user