Skip to content

Commit

Permalink
backend: pass Index and keep_newer timestamp parameters to gc()
Browse files Browse the repository at this point in the history
GitBackend::gc() will need to check if a commit is reachable from any
historical operations. This could be calculated from the view and commit
objects, but the Index will do a better job.
  • Loading branch information
yuja committed Jan 17, 2024
1 parent 97f5f8e commit 8681e7c
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 10 deletions.
6 changes: 4 additions & 2 deletions cli/examples/custom-backend/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use std::any::Any;
use std::io::Read;
use std::path::Path;
use std::time::SystemTime;

use async_trait::async_trait;
use jj_cli::cli_util::{CliRunner, CommandError, CommandHelper};
Expand All @@ -24,6 +25,7 @@ use jj_lib::backend::{
Conflict, ConflictId, FileId, SigningFn, SymlinkId, Tree, TreeId,
};
use jj_lib::git_backend::GitBackend;
use jj_lib::index::Index;
use jj_lib::repo::StoreFactories;
use jj_lib::repo_path::RepoPath;
use jj_lib::settings::UserSettings;
Expand Down Expand Up @@ -171,7 +173,7 @@ impl Backend for JitBackend {
self.inner.write_commit(contents, sign_with)
}

fn gc(&self) -> BackendResult<()> {
self.inner.gc()
fn gc(&self, index: &dyn Index, keep_newer: SystemTime) -> BackendResult<()> {
self.inner.gc(index, keep_newer)
}
}
2 changes: 1 addition & 1 deletion cli/src/commands/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn cmd_util_gc(
let repo = workspace_command.repo();
repo.op_store()
.gc(slice::from_ref(repo.op_id()), keep_newer)?;
repo.store().gc()?;
repo.store().gc(repo.index(), keep_newer)?;
Ok(())
}

Expand Down
9 changes: 7 additions & 2 deletions lib/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ use std::collections::BTreeMap;
use std::fmt::Debug;
use std::io::Read;
use std::result::Result;
use std::time::SystemTime;
use std::vec::Vec;

use async_trait::async_trait;
use thiserror::Error;

use crate::content_hash::ContentHash;
use crate::index::Index;
use crate::merge::Merge;
use crate::object_id::{id_type, ObjectId};
use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathComponentBuf};
Expand Down Expand Up @@ -450,6 +452,9 @@ pub trait Backend: Send + Sync + Debug {
) -> BackendResult<(CommitId, Commit)>;

/// Perform garbage collection.
// TODO: pass in the set of commits to keep here
fn gc(&self) -> BackendResult<()>;
///
/// All commits found in the `index` won't be removed. In addition to that,
/// objects created after `keep_newer` will be preserved. This mitigates a
/// risk of deleting new commits created concurrently by another process.
fn gc(&self, index: &dyn Index, keep_newer: SystemTime) -> BackendResult<()>;
}
5 changes: 4 additions & 1 deletion lib/src/git_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::io::{Cursor, Read};
use std::path::{Path, PathBuf};
use std::process::ExitStatus;
use std::sync::{Arc, Mutex, MutexGuard};
use std::time::SystemTime;
use std::{fs, io, str};

use async_trait::async_trait;
Expand All @@ -36,6 +37,7 @@ use crate::backend::{
TreeValue,
};
use crate::file_util::{IoResultExt as _, PathError};
use crate::index::Index;
use crate::lock::FileLock;
use crate::merge::{Merge, MergeBuilder};
use crate::object_id::ObjectId;
Expand Down Expand Up @@ -1077,7 +1079,8 @@ impl Backend for GitBackend {
Ok((id, contents))
}

fn gc(&self) -> BackendResult<()> {
fn gc(&self, _index: &dyn Index, _keep_newer: SystemTime) -> BackendResult<()> {
// TODO: pass in keep_newer to "git gc" command
run_git_gc(self.git_repo_path()).map_err(|err| BackendError::Other(err.into()))
}
}
Expand Down
4 changes: 3 additions & 1 deletion lib/src/local_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::fs;
use std::fs::File;
use std::io::{Read, Write};
use std::path::{Path, PathBuf};
use std::time::SystemTime;

use async_trait::async_trait;
use blake2::{Blake2b512, Digest};
Expand All @@ -33,6 +34,7 @@ use crate::backend::{
};
use crate::content_hash::blake2b_hash;
use crate::file_util::persist_content_addressed_temp_file;
use crate::index::Index;
use crate::merge::MergeBuilder;
use crate::object_id::ObjectId;
use crate::repo_path::{RepoPath, RepoPathComponentBuf};
Expand Down Expand Up @@ -299,7 +301,7 @@ impl Backend for LocalBackend {
Ok((id, commit))
}

fn gc(&self) -> BackendResult<()> {
fn gc(&self, _index: &dyn Index, _keep_newer: SystemTime) -> BackendResult<()> {
Ok(())
}
}
Expand Down
6 changes: 4 additions & 2 deletions lib/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::collections::HashMap;
use std::fmt::{Debug, Formatter};
use std::io::Read;
use std::sync::{Arc, RwLock};
use std::time::SystemTime;

use pollster::FutureExt;

Expand All @@ -27,6 +28,7 @@ use crate::backend::{
SymlinkId, TreeId,
};
use crate::commit::Commit;
use crate::index::Index;
use crate::merge::{Merge, MergedTreeValue};
use crate::merged_tree::MergedTree;
use crate::repo_path::{RepoPath, RepoPathBuf};
Expand Down Expand Up @@ -266,7 +268,7 @@ impl Store {
TreeBuilder::new(self.clone(), base_tree_id)
}

pub fn gc(&self) -> BackendResult<()> {
self.backend.gc()
pub fn gc(&self, index: &dyn Index, keep_newer: SystemTime) -> BackendResult<()> {
self.backend.gc(index, keep_newer)
}
}
4 changes: 3 additions & 1 deletion lib/testutils/src/test_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ use std::fmt::{Debug, Error, Formatter};
use std::io::{Cursor, Read};
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex, MutexGuard, OnceLock};
use std::time::SystemTime;

use async_trait::async_trait;
use jj_lib::backend::{
make_root_commit, Backend, BackendError, BackendResult, ChangeId, Commit, CommitId, Conflict,
ConflictId, FileId, SecureSig, SigningFn, SymlinkId, Tree, TreeId,
};
use jj_lib::index::Index;
use jj_lib::object_id::ObjectId;
use jj_lib::repo_path::{RepoPath, RepoPathBuf};

Expand Down Expand Up @@ -298,7 +300,7 @@ impl Backend for TestBackend {
Ok((id, contents))
}

fn gc(&self) -> BackendResult<()> {
fn gc(&self, _index: &dyn Index, _keep_newer: SystemTime) -> BackendResult<()> {
Ok(())
}
}

0 comments on commit 8681e7c

Please sign in to comment.