Skip to content

Commit

Permalink
simple_op_store: replace Protobuf by Thrift
Browse files Browse the repository at this point in the history
As mentioned in the previous commit, we need to remove the Protobuf
dependency in order to be allowed to import jj into Google's
repo. This commit makes `SimpleOpStore` store its data using Thrift
instead of Protobufs. It also adds automatic upgrade of existing
repos. The upgrade process took 18 s in my repo, which has 22k
operations. The upgraded storage uses practically the same amount of
space. `jj op log` (the full outut) in my repo slowed down from 1.2 s
to 3.4 s. Luckily that's an uncommon operation. I couldn't measure any
difference in `jj status` (loading a single operation).
  • Loading branch information
martinvonz committed Nov 9, 2022
1 parent baf3c01 commit da38602
Show file tree
Hide file tree
Showing 9 changed files with 2,329 additions and 18 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
description, even if there already was a description set. It now also only
works on the working-copy commit (there's no `-r` argument).

* The storage format for the operation log has changed. It will be
automatically upgraded the first time you run a command in an existing repo.
The operation IDs will change in that process.

### New features

* The new `jj git remote rename` command allows git remotes to be renamed
Expand Down
26 changes: 26 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@

cargoLock = {
lockFile = "${self}/Cargo.lock";
outputHashes = {
"thrift-0.17.0" = "sha256-Zczwq6zRKPXXj7JR0X/0Osl1Lafo5r+2wK5tuWJbvI8=";
};
};
nativeBuildInputs = [
pkg-config gzip makeWrapper
Expand Down
6 changes: 6 additions & 0 deletions lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ regex = "1.7.0"
serde_json = "1.0.87"
tempfile = "3.3.0"
thiserror = "1.0.37"
# thrift v0.17.0 (specified by hash for security reasons)
thrift = { git = "https://github.com/apache/thrift", rev = "4d493e867b349f3475203ef9848353b315203c51", default-features = false }
uuid = { version = "1.2.1", features = ["v4"] }
whoami = "1.2.3"
zstd = "0.11.2"
Expand All @@ -49,4 +51,8 @@ test-case = "2.2.2"
testutils = { path = "testutils" }

[features]
default = ["legacy_protobuf"]
vendored-openssl = ["git2/vendored-openssl"]
# Enable upgrade of repositories created with storage backends based on
# Protobuf format (from before we switched to Thrift)
legacy_protobuf = []
3 changes: 3 additions & 0 deletions lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub mod nightly_shims;
pub mod op_heads_store;
pub mod op_store;
pub mod operation;
#[cfg(feature = "legacy_protobuf")]
mod proto_op_store;
pub mod protos;
pub mod refs;
Expand All @@ -44,6 +45,8 @@ pub mod revset_graph_iterator;
pub mod rewrite;
pub mod settings;
pub mod simple_op_store;
#[allow(dead_code)]
mod simple_op_store_model;
pub mod stacked_table;
pub mod store;
pub mod transaction;
Expand Down
17 changes: 3 additions & 14 deletions lib/src/proto_op_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use std::path::PathBuf;
use blake2::{Blake2b512, Digest};
use itertools::Itertools;
use protobuf::{Message, MessageField};
use tempfile::{NamedTempFile, PersistError};
use tempfile::NamedTempFile;

use crate::backend::{CommitId, MillisSinceEpoch, Timestamp};
use crate::file_util::persist_content_addressed_temp_file;
Expand All @@ -31,18 +31,6 @@ use crate::op_store::{
RefTarget, View, ViewId, WorkspaceId,
};

impl From<std::io::Error> for OpStoreError {
fn from(err: std::io::Error) -> Self {
OpStoreError::Other(err.to_string())
}
}

impl From<PersistError> for OpStoreError {
fn from(err: PersistError) -> Self {
OpStoreError::Other(err.to_string())
}
}

impl From<protobuf::Error> for OpStoreError {
fn from(err: protobuf::Error) -> Self {
OpStoreError::Other(err.to_string())
Expand All @@ -55,7 +43,8 @@ pub struct ProtoOpStore {
}

impl ProtoOpStore {
pub fn init(store_path: PathBuf) -> Self {
#[allow(dead_code)]
fn init(store_path: PathBuf) -> Self {
fs::create_dir(store_path.join("views")).unwrap();
fs::create_dir(store_path.join("operations")).unwrap();
Self::load(store_path)
Expand Down
Loading

0 comments on commit da38602

Please sign in to comment.