ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

backend: make write methods async

Browse source 
This doesn't provide any benefit yet bit I think we've known for a
while that we want to make the backend write methods async. It's just
not been important to Google because we have the local daemon process
that makes our writes pretty fast. Regardless, this first commit just
changes the API and all callers immediately block for now, so it won't
help even on slow backends.
This commit is contained in:
Martin von Zweigbergk 2024-08-20 10:33:54 -07:00 committed by Martin von Zweigbergk
parent 3fcb6eedfd
commit 8eb3d85b1c
7 changed files with 104 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
cli/examples/custom-backend/main.rs
View file

@ -149,24 +149,28 @@ impl Backend for JitBackend {
self.inner.read_file(path, id).await
}
fn write_file(&self, path: &RepoPath, contents: &mut dyn Read) -> BackendResult<FileId> {
self.inner.write_file(path, contents)
async fn write_file(
&self,
path: &RepoPath,
contents: &mut (dyn Read + Send),
) -> BackendResult<FileId> {
self.inner.write_file(path, contents).await
}
async fn read_symlink(&self, path: &RepoPath, id: &SymlinkId) -> BackendResult<String> {
self.inner.read_symlink(path, id).await
}
fn write_symlink(&self, path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
self.inner.write_symlink(path, target)
async fn write_symlink(&self, path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
self.inner.write_symlink(path, target).await
}
async fn read_tree(&self, path: &RepoPath, id: &TreeId) -> BackendResult<Tree> {
self.inner.read_tree(path, id).await
}
fn write_tree(&self, path: &RepoPath, contents: &Tree) -> BackendResult<TreeId> {
self.inner.write_tree(path, contents)
async fn write_tree(&self, path: &RepoPath, contents: &Tree) -> BackendResult<TreeId> {
self.inner.write_tree(path, contents).await
}
fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
@ -181,12 +185,12 @@ impl Backend for JitBackend {
self.inner.read_commit(id).await
}
fn write_commit(
async fn write_commit(
&self,
contents: Commit,
sign_with: Option<&mut SigningFn>,
) -> BackendResult<(CommitId, Commit)> {
self.inner.write_commit(contents, sign_with)
self.inner.write_commit(contents, sign_with).await
}
fn get_copy_records(

14
lib/src/backend.rs
View file

@ -90,7 +90,7 @@ pub struct SecureSig {
pub sig: Vec<u8>,
}
pub type SigningFn<'a> = dyn FnMut(&[u8]) -> SignResult<Vec<u8>> + 'a;
pub type SigningFn<'a> = dyn FnMut(&[u8]) -> SignResult<Vec<u8>> + Send + 'a;
/// Identifies a single legacy tree, which may have path-level conflicts, or a
/// merge of multiple trees, where the individual trees do not have conflicts.
@ -411,15 +411,19 @@ pub trait Backend: Send + Sync + Debug {
async fn read_file(&self, path: &RepoPath, id: &FileId) -> BackendResult<Box<dyn Read>>;
fn write_file(&self, path: &RepoPath, contents: &mut dyn Read) -> BackendResult<FileId>;
async fn write_file(
&self,
path: &RepoPath,
contents: &mut (dyn Read + Send),
) -> BackendResult<FileId>;
async fn read_symlink(&self, path: &RepoPath, id: &SymlinkId) -> BackendResult<String>;
fn write_symlink(&self, path: &RepoPath, target: &str) -> BackendResult<SymlinkId>;
async fn write_symlink(&self, path: &RepoPath, target: &str) -> BackendResult<SymlinkId>;
async fn read_tree(&self, path: &RepoPath, id: &TreeId) -> BackendResult<Tree>;
fn write_tree(&self, path: &RepoPath, contents: &Tree) -> BackendResult<TreeId>;
async fn write_tree(&self, path: &RepoPath, contents: &Tree) -> BackendResult<TreeId>;
// Not async because it would force `MergedTree::value()` to be async. We don't
// need this to be async anyway because it's only used by legacy repos.
@ -442,7 +446,7 @@ pub trait Backend: Send + Sync + Debug {
/// an implementation specific fashion, and both `read_commit` and the
/// return of `write_commit` should read it back as the `secure_sig`
/// field.
fn write_commit(
async fn write_commit(
&self,
contents: Commit,
sign_with: Option<&mut SigningFn>,

46
lib/src/git_backend.rs
View file

@ -930,7 +930,11 @@ impl Backend for GitBackend {
self.read_file_sync(id)
}
fn write_file(&self, _path: &RepoPath, contents: &mut dyn Read) -> BackendResult<FileId> {
async fn write_file(
&self,
_path: &RepoPath,
contents: &mut (dyn Read + Send),
) -> BackendResult<FileId> {
let mut bytes = Vec::new();
contents.read_to_end(&mut bytes).unwrap();
let locked_repo = self.lock_git_repo();
@ -957,7 +961,7 @@ impl Backend for GitBackend {
Ok(target)
}
fn write_symlink(&self, _path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
async fn write_symlink(&self, _path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
let locked_repo = self.lock_git_repo();
let oid =
locked_repo
@ -1032,7 +1036,7 @@ impl Backend for GitBackend {
Ok(tree)
}
fn write_tree(&self, _path: &RepoPath, contents: &Tree) -> BackendResult<TreeId> {
async fn write_tree(&self, _path: &RepoPath, contents: &Tree) -> BackendResult<TreeId> {
// Tree entries to be written must be sorted by Entry::filename(), which
// is slightly different from the order of our backend::Tree.
let entries = contents
@ -1161,7 +1165,7 @@ impl Backend for GitBackend {
Ok(commit)
}
fn write_commit(
async fn write_commit(
&self,
mut contents: Commit,
mut sign_with: Option<&mut SigningFn>,
@ -1830,16 +1834,20 @@ mod tests {
secure_sig: None,
};
let write_commit = |commit: Commit| -> BackendResult<(CommitId, Commit)> {
backend.write_commit(commit, None).block_on()
};
// No parents
commit.parents = vec![];
assert_matches!(
backend.write_commit(commit.clone(), None),
write_commit(commit.clone()),
Err(BackendError::Other(err)) if err.to_string().contains("no parents")
);
// Only root commit as parent
commit.parents = vec![backend.root_commit_id().clone()];
let first_id = backend.write_commit(commit.clone(), None).unwrap().0;
let first_id = write_commit(commit.clone()).unwrap().0;
let first_commit = backend.read_commit(&first_id).block_on().unwrap();
assert_eq!(first_commit, commit);
let first_git_commit = git_repo.find_commit(git_id(&first_id)).unwrap();
@ -1847,7 +1855,7 @@ mod tests {
// Only non-root commit as parent
commit.parents = vec![first_id.clone()];
let second_id = backend.write_commit(commit.clone(), None).unwrap().0;
let second_id = write_commit(commit.clone()).unwrap().0;
let second_commit = backend.read_commit(&second_id).block_on().unwrap();
assert_eq!(second_commit, commit);
let second_git_commit = git_repo.find_commit(git_id(&second_id)).unwrap();
@ -1858,7 +1866,7 @@ mod tests {
// Merge commit
commit.parents = vec![first_id.clone(), second_id.clone()];
let merge_id = backend.write_commit(commit.clone(), None).unwrap().0;
let merge_id = write_commit(commit.clone()).unwrap().0;
let merge_commit = backend.read_commit(&merge_id).block_on().unwrap();
assert_eq!(merge_commit, commit);
let merge_git_commit = git_repo.find_commit(git_id(&merge_id)).unwrap();
@ -1870,7 +1878,7 @@ mod tests {
// Merge commit with root as one parent
commit.parents = vec![first_id, backend.root_commit_id().clone()];
assert_matches!(
backend.write_commit(commit, None),
write_commit(commit),
Err(BackendError::Unsupported(message)) if message.contains("root commit")
);
}
@ -1908,9 +1916,13 @@ mod tests {
secure_sig: None,
};
let write_commit = |commit: Commit| -> BackendResult<(CommitId, Commit)> {
backend.write_commit(commit, None).block_on()
};
// When writing a tree-level conflict, the root tree on the git side has the
// individual trees as subtrees.
let read_commit_id = backend.write_commit(commit.clone(), None).unwrap().0;
let read_commit_id = write_commit(commit.clone()).unwrap().0;
let read_commit = backend.read_commit(&read_commit_id).block_on().unwrap();
assert_eq!(read_commit, commit);
let git_commit = git_repo
@ -1960,7 +1972,7 @@ mod tests {
// When writing a single tree using the new format, it's represented by a
// regular git tree.
commit.root_tree = MergedTreeId::resolved(create_tree(5));
let read_commit_id = backend.write_commit(commit.clone(), None).unwrap().0;
let read_commit_id = write_commit(commit.clone()).unwrap().0;
let read_commit = backend.read_commit(&read_commit_id).block_on().unwrap();
assert_eq!(read_commit, commit);
let git_commit = git_repo
@ -1996,7 +2008,7 @@ mod tests {
committer: signature,
secure_sig: None,
};
let commit_id = backend.write_commit(commit, None).unwrap().0;
let commit_id = backend.write_commit(commit, None).block_on().unwrap().0;
let git_refs: Vec<_> = git_repo
.references_glob("refs/jj/keep/*")
.unwrap()
@ -2073,15 +2085,20 @@ mod tests {
committer: create_signature(),
secure_sig: None,
};
let write_commit = |commit: Commit| -> BackendResult<(CommitId, Commit)> {
backend.write_commit(commit, None).block_on()
};
// libgit2 doesn't seem to preserve negative timestamps, so set it to at least 1
// second after the epoch, so the timestamp adjustment can remove 1
// second and it will still be nonnegative
commit1.committer.timestamp.timestamp = MillisSinceEpoch(1000);
let (commit_id1, mut commit2) = backend.write_commit(commit1, None).unwrap();
let (commit_id1, mut commit2) = write_commit(commit1).unwrap();
commit2.predecessors.push(commit_id1.clone());
// `write_commit` should prevent the ids from being the same by changing the
// committer timestamp of the commit it actually writes.
let (commit_id2, mut actual_commit2) = backend.write_commit(commit2.clone(), None).unwrap();
let (commit_id2, mut actual_commit2) = write_commit(commit2.clone()).unwrap();
// The returned matches the ID
assert_eq!(
backend.read_commit(&commit_id2).block_on().unwrap(),
@ -2122,6 +2139,7 @@ mod tests {
let (id, commit) = backend
.write_commit(commit, Some(&mut signer as &mut SigningFn))
.block_on()
.unwrap();
let git_repo = backend.git_repo();

32
lib/src/local_backend.rs
View file

@ -29,6 +29,7 @@ use blake2::Blake2b512;
use blake2::Digest;
use futures::stream;
use futures::stream::BoxStream;
use pollster::FutureExt;
use prost::Message;
use tempfile::NamedTempFile;
@ -108,6 +109,7 @@ impl LocalBackend {
let backend = Self::load(store_path);
let empty_tree_id = backend
.write_tree(RepoPath::root(), &Tree::default())
.block_on()
.unwrap();
assert_eq!(empty_tree_id, backend.empty_tree_id);
backend
@ -116,7 +118,9 @@ impl LocalBackend {
pub fn load(store_path: &Path) -> Self {
let root_commit_id = CommitId::from_bytes(&[0; COMMIT_ID_LENGTH]);
let root_change_id = ChangeId::from_bytes(&[0; CHANGE_ID_LENGTH]);
let empty_tree_id = TreeId::from_hex("482ae5a29fbe856c7272f2071b8b0f0359ee2d89ff392b8a900643fbd0836eccd067b8bf41909e206c90d45d6e7d8b6686b93ecaee5fe1a9060d87b672101310");
let empty_tree_id = TreeId::from_hex(
"482ae5a29fbe856c7272f2071b8b0f0359ee2d89ff392b8a900643fbd0836eccd067b8bf41909e206c90d45d6e7d8b6686b93ecaee5fe1a9060d87b672101310",
);
LocalBackend {
path: store_path.to_path_buf(),
root_commit_id,
@ -186,7 +190,11 @@ impl Backend for LocalBackend {
Ok(Box::new(zstd::Decoder::new(file).map_err(to_other_err)?))
}
fn write_file(&self, _path: &RepoPath, contents: &mut dyn Read) -> BackendResult<FileId> {
async fn write_file(
&self,
_path: &RepoPath,
contents: &mut (dyn Read + Send),
) -> BackendResult<FileId> {
let temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?;
let mut encoder = zstd::Encoder::new(temp_file.as_file(), 0).map_err(to_other_err)?;
let mut hasher = Blake2b512::new();
@ -214,7 +222,7 @@ impl Backend for LocalBackend {
Ok(target)
}
fn write_symlink(&self, _path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
async fn write_symlink(&self, _path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
let mut temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?;
temp_file
.write_all(target.as_bytes())
@ -236,7 +244,7 @@ impl Backend for LocalBackend {
Ok(tree_from_proto(proto))
}
fn write_tree(&self, _path: &RepoPath, tree: &Tree) -> BackendResult<TreeId> {
async fn write_tree(&self, _path: &RepoPath, tree: &Tree) -> BackendResult<TreeId> {
let temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?;
let proto = tree_to_proto(tree);
@ -291,7 +299,7 @@ impl Backend for LocalBackend {
Ok(commit_from_proto(proto))
}
fn write_commit(
async fn write_commit(
&self,
mut commit: Commit,
sign_with: Option<&mut SigningFn>,
@ -550,34 +558,38 @@ mod tests {
secure_sig: None,
};
let write_commit = |commit: Commit| -> BackendResult<(CommitId, Commit)> {
backend.write_commit(commit, None).block_on()
};
// No parents
commit.parents = vec![];
assert_matches!(
backend.write_commit(commit.clone(), None),
write_commit(commit.clone()),
Err(BackendError::Other(err)) if err.to_string().contains("no parents")
);
// Only root commit as parent
commit.parents = vec![backend.root_commit_id().clone()];
let first_id = backend.write_commit(commit.clone(), None).unwrap().0;
let first_id = write_commit(commit.clone()).unwrap().0;
let first_commit = backend.read_commit(&first_id).block_on().unwrap();
assert_eq!(first_commit, commit);
// Only non-root commit as parent
commit.parents = vec![first_id.clone()];
let second_id = backend.write_commit(commit.clone(), None).unwrap().0;
let second_id = write_commit(commit.clone()).unwrap().0;
let second_commit = backend.read_commit(&second_id).block_on().unwrap();
assert_eq!(second_commit, commit);
// Merge commit
commit.parents = vec![first_id.clone(), second_id.clone()];
let merge_id = backend.write_commit(commit.clone(), None).unwrap().0;
let merge_id = write_commit(commit.clone()).unwrap().0;
let merge_commit = backend.read_commit(&merge_id).block_on().unwrap();
assert_eq!(merge_commit, commit);
// Merge commit with root as one parent
commit.parents = vec![first_id, backend.root_commit_id().clone()];
let root_merge_id = backend.write_commit(commit.clone(), None).unwrap().0;
let root_merge_id = write_commit(commit.clone()).unwrap().0;
let root_merge_commit = backend.read_commit(&root_merge_id).block_on().unwrap();
assert_eq!(root_merge_commit, commit);
}

20
lib/src/secret_backend.rs
View file

@ -131,8 +131,12 @@ impl Backend for SecretBackend {
self.inner.read_file(path, id).await
}
fn write_file(&self, path: &RepoPath, contents: &mut dyn Read) -> BackendResult<FileId> {
self.inner.write_file(path, contents)
async fn write_file(
&self,
path: &RepoPath,
contents: &mut (dyn Read + Send),
) -> BackendResult<FileId> {
self.inner.write_file(path, contents).await
}
async fn read_symlink(&self, path: &RepoPath, id: &SymlinkId) -> BackendResult<String> {
@ -148,16 +152,16 @@ impl Backend for SecretBackend {
self.inner.read_symlink(path, id).await
}
fn write_symlink(&self, path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
self.inner.write_symlink(path, target)
async fn write_symlink(&self, path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
self.inner.write_symlink(path, target).await
}
async fn read_tree(&self, path: &RepoPath, id: &TreeId) -> BackendResult<Tree> {
self.inner.read_tree(path, id).await
}
fn write_tree(&self, path: &RepoPath, contents: &Tree) -> BackendResult<TreeId> {
self.inner.write_tree(path, contents)
async fn write_tree(&self, path: &RepoPath, contents: &Tree) -> BackendResult<TreeId> {
self.inner.write_tree(path, contents).await
}
fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
@ -172,12 +176,12 @@ impl Backend for SecretBackend {
self.inner.read_commit(id).await
}
fn write_commit(
async fn write_commit(
&self,
contents: Commit,
sign_with: Option<&mut SigningFn>,
) -> BackendResult<(CommitId, Commit)> {
self.inner.write_commit(contents, sign_with)
self.inner.write_commit(contents, sign_with).await
}
fn get_copy_records(

14
lib/src/store.rs
View file

@ -160,7 +160,7 @@ impl Store {
) -> BackendResult<Commit> {
assert!(!commit.parents.is_empty());
let (commit_id, commit) = self.backend.write_commit(commit, sign_with)?;
let (commit_id, commit) = self.backend.write_commit(commit, sign_with).block_on()?;
let data = Arc::new(commit);
{
let mut locked_cache = self.commit_cache.lock().unwrap();
@ -220,7 +220,7 @@ impl Store {
path: &RepoPath,
tree: backend::Tree,
) -> BackendResult<Tree> {
let tree_id = self.backend.write_tree(path, &tree)?;
let tree_id = self.backend.write_tree(path, &tree).block_on()?;
let data = Arc::new(tree);
{
let mut locked_cache = self.tree_cache.lock().unwrap();
@ -242,8 +242,12 @@ impl Store {
self.backend.read_file(path, id).await
}
pub fn write_file(&self, path: &RepoPath, contents: &mut dyn Read) -> BackendResult<FileId> {
self.backend.write_file(path, contents)
pub fn write_file(
&self,
path: &RepoPath,
contents: &mut (dyn Read + Send),
) -> BackendResult<FileId> {
self.backend.write_file(path, contents).block_on()
}
pub fn read_symlink(&self, path: &RepoPath, id: &SymlinkId) -> BackendResult<String> {
@ -259,7 +263,7 @@ impl Store {
}
pub fn write_symlink(&self, path: &RepoPath, contents: &str) -> BackendResult<SymlinkId> {
self.backend.write_symlink(path, contents)
self.backend.write_symlink(path, contents).block_on()
}
pub fn read_conflict(

12
lib/testutils/src/test_backend.rs
View file

@ -189,7 +189,11 @@ impl Backend for TestBackend {
}
}
fn write_file(&self, path: &RepoPath, contents: &mut dyn Read) -> BackendResult<FileId> {
async fn write_file(
&self,
path: &RepoPath,
contents: &mut (dyn Read + Send),
) -> BackendResult<FileId> {
let mut bytes = Vec::new();
contents.read_to_end(&mut bytes).unwrap();
let id = FileId::new(get_hash(&bytes));
@ -218,7 +222,7 @@ impl Backend for TestBackend {
}
}
fn write_symlink(&self, path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
async fn write_symlink(&self, path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
let id = SymlinkId::new(get_hash(target.as_bytes()));
self.locked_data()
.symlinks
@ -248,7 +252,7 @@ impl Backend for TestBackend {
}
}
fn write_tree(&self, path: &RepoPath, contents: &Tree) -> BackendResult<TreeId> {
async fn write_tree(&self, path: &RepoPath, contents: &Tree) -> BackendResult<TreeId> {
let id = TreeId::new(get_hash(contents));
self.locked_data()
.trees
@ -302,7 +306,7 @@ impl Backend for TestBackend {
}
}
fn write_commit(
async fn write_commit(
&self,
mut contents: Commit,
mut sign_with: Option<&mut SigningFn>,