Skip to content

Commit

Permalink
fix: bump cache sqlite dbs to v2 for WAL journal mode change (denolan…
Browse files Browse the repository at this point in the history
…d#24030)

In denoland#23955 we changed the sqlite db
journal mode to WAL. This causes issues when someone is running an old
version of Deno using TRUNCATE and a new version because the two fight
against each other.
  • Loading branch information
dsherret committed May 29, 2024
1 parent fada25b commit 94f040a
Show file tree
Hide file tree
Showing 22 changed files with 347 additions and 263 deletions.
8 changes: 4 additions & 4 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ winres.workspace = true
[dependencies]
deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] }
deno_cache_dir = { workspace = true }
deno_config = "=0.16.3"
deno_config = "=0.16.4"
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_doc = { version = "=0.137.0", features = ["html", "syntect"] }
deno_emit = "=0.41.0"
deno_graph = { version = "=0.77.0", features = ["tokio_executor"] }
deno_graph = { version = "=0.77.2", features = ["tokio_executor"] }
deno_lint = { version = "=0.58.4", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm = "=0.21.0"
Expand Down
95 changes: 80 additions & 15 deletions cli/cache/cache_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,48 @@ use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;

use super::FastInsecureHasher;

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct CacheDBHash(u64);

impl CacheDBHash {
pub fn new(hash: u64) -> Self {
Self(hash)
}

pub fn from_source(source: impl std::hash::Hash) -> Self {
Self::new(
// always write in the deno version just in case
// the clearing on deno version change doesn't work
FastInsecureHasher::new_deno_versioned()
.write_hashable(source)
.finish(),
)
}
}

impl rusqlite::types::ToSql for CacheDBHash {
fn to_sql(&self) -> rusqlite::Result<rusqlite::types::ToSqlOutput<'_>> {
Ok(rusqlite::types::ToSqlOutput::Owned(
// sqlite doesn't support u64, but it does support i64 so store
// this value "incorrectly" as i64 then convert back to u64 on read
rusqlite::types::Value::Integer(self.0 as i64),
))
}
}

impl rusqlite::types::FromSql for CacheDBHash {
fn column_result(
value: rusqlite::types::ValueRef,
) -> rusqlite::types::FromSqlResult<Self> {
match value {
rusqlite::types::ValueRef::Integer(i) => Ok(Self::new(i as u64)),
_ => Err(rusqlite::types::FromSqlError::InvalidType),
}
}
}

/// What should the cache should do on failure?
#[derive(Default)]
pub enum CacheFailure {
Expand Down Expand Up @@ -41,21 +83,16 @@ pub struct CacheDBConfiguration {
impl CacheDBConfiguration {
fn create_combined_sql(&self) -> String {
format!(
"
PRAGMA journal_mode=WAL;
PRAGMA synchronous=NORMAL;
PRAGMA temp_store=memory;
PRAGMA page_size=4096;
PRAGMA mmap_size=6000000;
PRAGMA optimize;
CREATE TABLE IF NOT EXISTS info (
key TEXT PRIMARY KEY,
value TEXT NOT NULL
);
{}
",
concat!(
"PRAGMA journal_mode=WAL;",
"PRAGMA synchronous=NORMAL;",
"PRAGMA temp_store=memory;",
"PRAGMA page_size=4096;",
"PRAGMA mmap_size=6000000;",
"PRAGMA optimize;",
"CREATE TABLE IF NOT EXISTS info (key TEXT PRIMARY KEY, value TEXT NOT NULL);",
"{}",
),
self.table_initializer
)
}
Expand Down Expand Up @@ -520,4 +557,32 @@ mod tests {
})
.expect_err("Should have failed");
}

#[test]
fn cache_db_hash_max_u64_value() {
assert_same_serialize_deserialize(CacheDBHash::new(u64::MAX));
assert_same_serialize_deserialize(CacheDBHash::new(u64::MAX - 1));
assert_same_serialize_deserialize(CacheDBHash::new(u64::MIN));
assert_same_serialize_deserialize(CacheDBHash::new(u64::MIN + 1));
}

fn assert_same_serialize_deserialize(original_hash: CacheDBHash) {
use rusqlite::types::FromSql;
use rusqlite::types::ValueRef;
use rusqlite::ToSql;

let value = original_hash.to_sql().unwrap();
match value {
rusqlite::types::ToSqlOutput::Owned(rusqlite::types::Value::Integer(
value,
)) => {
let value_ref = ValueRef::Integer(value);
assert_eq!(
original_hash,
CacheDBHash::column_result(value_ref).unwrap()
);
}
_ => unreachable!(),
}
}
}
55 changes: 31 additions & 24 deletions cli/cache/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,21 @@

use super::cache_db::CacheDB;
use super::cache_db::CacheDBConfiguration;
use super::cache_db::CacheDBHash;
use super::cache_db::CacheFailure;
use deno_ast::ModuleSpecifier;
use deno_core::error::AnyError;
use deno_runtime::deno_webstorage::rusqlite::params;

pub static TYPE_CHECK_CACHE_DB: CacheDBConfiguration = CacheDBConfiguration {
table_initializer: concat!(
"CREATE TABLE IF NOT EXISTS checkcache (
check_hash TEXT PRIMARY KEY
);",
"CREATE TABLE IF NOT EXISTS tsbuildinfo (
specifier TEXT PRIMARY KEY,
text TEXT NOT NULL
);",
"CREATE TABLE IF NOT EXISTS checkcache (",
"check_hash INT PRIMARY KEY",
");",
"CREATE TABLE IF NOT EXISTS tsbuildinfo (",
"specifier TEXT PRIMARY KEY,",
"text TEXT NOT NULL",
");",
),
on_version_change: concat!(
"DELETE FROM checkcache;",
Expand All @@ -37,7 +38,7 @@ impl TypeCheckCache {
Self(db)
}

pub fn has_check_hash(&self, hash: u64) -> bool {
pub fn has_check_hash(&self, hash: CacheDBHash) -> bool {
match self.hash_check_hash_result(hash) {
Ok(val) => val,
Err(err) => {
Expand All @@ -52,14 +53,17 @@ impl TypeCheckCache {
}
}

fn hash_check_hash_result(&self, hash: u64) -> Result<bool, AnyError> {
fn hash_check_hash_result(
&self,
hash: CacheDBHash,
) -> Result<bool, AnyError> {
self.0.exists(
"SELECT * FROM checkcache WHERE check_hash=?1 LIMIT 1",
params![hash.to_string()],
params![hash],
)
}

pub fn add_check_hash(&self, check_hash: u64) {
pub fn add_check_hash(&self, check_hash: CacheDBHash) {
if let Err(err) = self.add_check_hash_result(check_hash) {
if cfg!(debug_assertions) {
panic!("Error saving check hash: {err}");
Expand All @@ -69,13 +73,16 @@ impl TypeCheckCache {
}
}

fn add_check_hash_result(&self, check_hash: u64) -> Result<(), AnyError> {
fn add_check_hash_result(
&self,
check_hash: CacheDBHash,
) -> Result<(), AnyError> {
let sql = "
INSERT OR REPLACE INTO
checkcache (check_hash)
VALUES
(?1)";
self.0.execute(sql, params![&check_hash.to_string(),])?;
self.0.execute(sql, params![check_hash])?;
Ok(())
}

Expand Down Expand Up @@ -123,10 +130,10 @@ mod test {
let conn = CacheDB::in_memory(&TYPE_CHECK_CACHE_DB, "1.0.0");
let cache = TypeCheckCache::new(conn);

assert!(!cache.has_check_hash(1));
cache.add_check_hash(1);
assert!(cache.has_check_hash(1));
assert!(!cache.has_check_hash(2));
assert!(!cache.has_check_hash(CacheDBHash::new(1)));
cache.add_check_hash(CacheDBHash::new(1));
assert!(cache.has_check_hash(CacheDBHash::new(1)));
assert!(!cache.has_check_hash(CacheDBHash::new(2)));

let specifier1 = ModuleSpecifier::parse("file:https:///test.json").unwrap();
assert_eq!(cache.get_tsbuildinfo(&specifier1), None);
Expand All @@ -137,9 +144,9 @@ mod test {
let conn = cache.0.recreate_with_version("2.0.0");
let cache = TypeCheckCache::new(conn);

assert!(!cache.has_check_hash(1));
cache.add_check_hash(1);
assert!(cache.has_check_hash(1));
assert!(!cache.has_check_hash(CacheDBHash::new(1)));
cache.add_check_hash(CacheDBHash::new(1));
assert!(cache.has_check_hash(CacheDBHash::new(1)));
assert_eq!(cache.get_tsbuildinfo(&specifier1), None);
cache.set_tsbuildinfo(&specifier1, "test");
assert_eq!(cache.get_tsbuildinfo(&specifier1), Some("test".to_string()));
Expand All @@ -148,13 +155,13 @@ mod test {
let conn = cache.0.recreate_with_version("2.0.0");
let cache = TypeCheckCache::new(conn);

assert!(cache.has_check_hash(1));
assert!(!cache.has_check_hash(2));
assert!(cache.has_check_hash(CacheDBHash::new(1)));
assert!(!cache.has_check_hash(CacheDBHash::new(2)));
assert_eq!(cache.get_tsbuildinfo(&specifier1), Some("test".to_string()));

// adding when already exists should not cause issue
cache.add_check_hash(1);
assert!(cache.has_check_hash(1));
cache.add_check_hash(CacheDBHash::new(1));
assert!(cache.has_check_hash(CacheDBHash::new(1)));
cache.set_tsbuildinfo(&specifier1, "other");
assert_eq!(
cache.get_tsbuildinfo(&specifier1),
Expand Down
Loading

0 comments on commit 94f040a

Please sign in to comment.