From b089ade6aee50efe665df325b81b03ce6216251a Mon Sep 17 00:00:00 2001 From: Taddes Date: Thu, 13 Nov 2025 17:47:32 -0500 Subject: [PATCH 01/14] update types --- .../2025-10-20-155711_create_schema/up.sql | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql b/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql index 8093f03043..10fb6d0a5e 100644 --- a/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql +++ b/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql @@ -5,10 +5,7 @@ CREATE TABLE user_collections ( modified TIMESTAMP NOT NULL, count BIGINT, total_bytes BIGINT, - PRIMARY KEY ( - user_id, - collection_id - ) + PRIMARY KEY (user_id, collection_id) ); -- bsos table @@ -25,13 +22,7 @@ CREATE TABLE bsos ( collection_id, bso_id ), - FOREIGN KEY ( - user_id, - collection_id - ) REFERENCES user_collections ( - user_id, - collection_id - ) ON DELETE CASCADE + FOREIGN KEY (user_id, collection_id) REFERENCES user_collections (user_id, collection_id) ON DELETE CASCADE ); CREATE INDEX bsos_modified_idx ON bsos ( @@ -63,13 +54,7 @@ CREATE TABLE batches ( collection_id, batch_id ), - FOREIGN KEY ( - user_id, - collection_id - ) REFERENCES user_collections ( - user_id, - collection_id - ) ON DELETE CASCADE + FOREIGN KEY (user_id, collection_id) REFERENCES user_collections (user_id, collection_id) ON DELETE CASCADE ); CREATE INDEX batch_expiry_idx ON batches ( @@ -102,4 +87,4 @@ CREATE TABLE batch_bsos ( collection_id, batch_id ) ON DELETE CASCADE -); +); \ No newline at end of file From baec2ef5941dc8c8d8796660861b420cf8cc5e3d Mon Sep 17 00:00:00 2001 From: Taddes Date: Thu, 13 Nov 2025 17:47:57 -0500 Subject: [PATCH 02/14] docs for UserIdentifier --- syncstorage-db-common/src/lib.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/syncstorage-db-common/src/lib.rs b/syncstorage-db-common/src/lib.rs index ff109438ab..bed1f926fc 100644 --- a/syncstorage-db-common/src/lib.rs +++ b/syncstorage-db-common/src/lib.rs @@ -269,11 +269,14 @@ pub enum Sorting { #[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] pub struct UserIdentifier { - /// For MySQL database backends as the primary key + /// For MySQL/Postgres database backends as the primary key. pub legacy_id: u64, - /// For NoSQL database backends that require randomly distributed primary keys + /// For NoSQL database backends that require randomly distributed primary keys. pub fxa_uid: String, + /// Key identifier; part of the sync crypto context. pub fxa_kid: String, + /// Hash of the Firefox user ID associated with a Sync account. pub hashed_fxa_uid: String, + /// Hash of the device id metadata. pub hashed_device_id: String, } From 09fc93584cbd1e6bcdb34344afc2a07df3ae19e1 Mon Sep 17 00:00:00 2001 From: Taddes Date: Thu, 13 Nov 2025 17:48:17 -0500 Subject: [PATCH 03/14] PgDbSession coll lock type --- syncstorage-postgres/src/db/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/syncstorage-postgres/src/db/mod.rs b/syncstorage-postgres/src/db/mod.rs index f61f841c7a..29ba31b06e 100644 --- a/syncstorage-postgres/src/db/mod.rs +++ b/syncstorage-postgres/src/db/mod.rs @@ -53,9 +53,9 @@ struct PgDbSession { /// The "current time" on the server used for this session's operations. timestamp: SyncTimestamp, /// Cache of collection modified timestamps per (HawkIdentifier, collection_id). - coll_modified_cache: HashMap<(UserIdentifier, i32), SyncTimestamp>, + coll_modified_cache: HashMap<(u32, i32), SyncTimestamp>, /// Currently locked collections. - coll_locks: HashMap<(UserIdentifier, i32), CollectionLock>, + coll_locks: HashMap<(u32, i32), CollectionLock>, /// Whether a transaction was started (begin() called) in_transaction: bool, /// Boolean to identify if query in active transaction. From ada73def37cbda9781d7e0bea755ba4a8f255be0 Mon Sep 17 00:00:00 2001 From: Taddes Date: Thu, 13 Nov 2025 17:48:33 -0500 Subject: [PATCH 04/14] lock logic --- Cargo.lock | 158 +++++++++++++++++++++---- syncstorage-postgres/src/db/db_impl.rs | 62 +++++++++- 2 files changed, 195 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9b64dfa3c1..9f0215cdbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -435,6 +435,15 @@ version = "2.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34efbcccd345379ca2868b2b2c9d3782e9cc58ba87bc7d79d5b53d9c9ae6f25d" +[[package]] +name = "block-buffer" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4152116fd6e9dadb291ae18fc1ec3575ed6d84c29642d97890f4b4a3417297e4" +dependencies = [ + "generic-array", +] + [[package]] name = "block-buffer" version = "0.10.4" @@ -689,6 +698,16 @@ dependencies = [ "typenum", ] +[[package]] +name = "crypto-mac" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "25fab6889090c8133f3deb8f73ba3c65a7f456f66436fc012a1b1e272b1e103e" +dependencies = [ + "generic-array", + "subtle", +] + [[package]] name = "curl" version = "0.4.49" @@ -781,7 +800,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef552e6f588e446098f6ba40d89ac146c8c7b64aade83c051ee00bb5d2bc18d" dependencies = [ "serde 1.0.219", - "uuid", + "uuid 1.18.0", ] [[package]] @@ -890,13 +909,22 @@ dependencies = [ "syn", ] +[[package]] +name = "digest" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3dd60d1080a57a05ab032377049e0591415d2b31afd7028356dbf3cc6dcb066" +dependencies = [ + "generic-array", +] + [[package]] name = "digest" version = "0.10.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ - "block-buffer", + "block-buffer 0.10.4", "crypto-common", "subtle", ] @@ -1215,7 +1243,7 @@ dependencies = [ "chrono", "serde 1.0.219", "serde_json", - "uuid", + "uuid 1.18.0", ] [[package]] @@ -1363,7 +1391,17 @@ version = "0.12.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7b5f8eb2ad728638ea2c7d47a21db23b7b58a72ed6a38256b8a1849f15fbbdf7" dependencies = [ - "hmac", + "hmac 0.12.1", +] + +[[package]] +name = "hmac" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a2a2320eb7ec0ebe8da8f744d7812d9fc4cb4d09344ac01898dbcb6a20ae69b" +dependencies = [ + "crypto-mac", + "digest 0.9.0", ] [[package]] @@ -1372,7 +1410,7 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c49c37c09c17a53d937dfbb742eb3a961d65a994e6bcdcf37e7399d0cc8ab5e" dependencies = [ - "digest", + "digest 0.10.7", ] [[package]] @@ -1855,6 +1893,23 @@ dependencies = [ "redox_syscall", ] +[[package]] +name = "libsystemd" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f4f0b5b062ba67aa075e331de778082c09e66b5ef32970ea5a1e9c37c9555d1" +dependencies = [ + "hmac 0.11.0", + "libc", + "log", + "nix", + "once_cell", + "serde 1.0.219", + "sha2 0.9.9", + "thiserror 1.0.69", + "uuid 0.8.2", +] + [[package]] name = "libz-sys" version = "1.1.22" @@ -1940,7 +1995,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d89e7ee0cfbedfc4da3340218492196241d89eefb6dab27de5df917a6d2e78cf" dependencies = [ "cfg-if", - "digest", + "digest 0.10.7", ] [[package]] @@ -1949,6 +2004,15 @@ version = "2.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a282da65faaf38286cf3be983213fcf1d2e2a58700e808f83f4ea9a4804bc0" +[[package]] +name = "memoffset" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5aa361d4faea93603064a027415f07bd8e1d5c88c9fbf68bf56a285428fd79ce" +dependencies = [ + "autocfg", +] + [[package]] name = "memoffset" version = "0.9.1" @@ -2085,9 +2149,22 @@ dependencies = [ "serde 1.0.219", "serde_json", "sha1", - "sha2", + "sha2 0.10.9", "thiserror 2.0.16", - "uuid", + "uuid 1.18.0", +] + +[[package]] +name = "nix" +version = "0.23.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f3790c00a0150112de0f4cd161e3d7fc4b2d8a5542ffc35f099a2562aecb35c" +dependencies = [ + "bitflags 1.3.2", + "cc", + "cfg-if", + "libc", + "memoffset 0.6.5", ] [[package]] @@ -2185,6 +2262,12 @@ version = "1.70.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4895175b425cb1f87721b59f0f286c2092bd4af812243672510e1ac53e2e0ad" +[[package]] +name = "opaque-debug" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381" + [[package]] name = "openssl-probe" version = "0.1.6" @@ -2334,11 +2417,11 @@ dependencies = [ "byteorder", "bytes", "fallible-iterator", - "hmac", + "hmac 0.12.1", "md-5", "memchr", "rand 0.9.2", - "sha2", + "sha2 0.10.9", "stringprep", ] @@ -2433,7 +2516,7 @@ dependencies = [ "cfg-if", "indoc", "libc", - "memoffset", + "memoffset 0.9.1", "once_cell", "portable-atomic", "pyo3-build-config", @@ -2969,7 +3052,7 @@ dependencies = [ "thiserror 2.0.16", "time", "url", - "uuid", + "uuid 1.18.0", ] [[package]] @@ -3051,7 +3134,20 @@ checksum = "e3bf829a2d51ab4a5ddf1352d8470c140cadc8301b2ae1789db023f01cedd6ba" dependencies = [ "cfg-if", "cpufeatures", - "digest", + "digest 0.10.7", +] + +[[package]] +name = "sha2" +version = "0.9.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4d58a1e1bf39749807d89cf2d98ac2dfa0ff1cb3faa38fbb64dd88ac8013d800" +dependencies = [ + "block-buffer 0.9.0", + "cfg-if", + "cpufeatures", + "digest 0.9.0", + "opaque-debug", ] [[package]] @@ -3062,7 +3158,7 @@ checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" dependencies = [ "cfg-if", "cpufeatures", - "digest", + "digest 0.10.7", ] [[package]] @@ -3134,6 +3230,16 @@ dependencies = [ "slog-term", ] +[[package]] +name = "slog-journald" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "83e14eb8c2f5d0c8fc9fbac40e6391095e4dc5cb334f7dce99c75cb1919eb39c" +dependencies = [ + "libsystemd", + "slog", +] + [[package]] name = "slog-mozlog-json" version = "0.1.0" @@ -3292,7 +3398,7 @@ dependencies = [ "glean", "hawk", "hex", - "hmac", + "hmac 0.12.1", "hostname", "http 1.3.1", "lazy_static", @@ -3302,10 +3408,11 @@ dependencies = [ "sentry", "serde 1.0.219", "serde_json", - "sha2", + "sha2 0.10.9", "slog", "slog-async", "slog-envlogger", + "slog-journald", "slog-mozlog-json", "slog-scope", "slog-stdlog", @@ -3342,7 +3449,7 @@ dependencies = [ "sentry", "sentry-backtrace", "serde_json", - "sha2", + "sha2 0.10.9", "slog", "slog-scope", ] @@ -3459,7 +3566,7 @@ dependencies = [ "thiserror 1.0.69", "tokio", "url", - "uuid", + "uuid 1.18.0", ] [[package]] @@ -3493,7 +3600,7 @@ dependencies = [ "syncstorage-settings", "thiserror 1.0.69", "tokio", - "uuid", + "uuid 1.18.0", ] [[package]] @@ -3660,7 +3767,7 @@ dependencies = [ "dyn-clone", "hex", "hkdf", - "hmac", + "hmac 0.12.1", "jsonwebtoken", "mockito", "pyo3", @@ -3668,7 +3775,7 @@ dependencies = [ "ring", "serde 1.0.219", "serde_json", - "sha2", + "sha2 0.10.9", "slog-scope", "syncserver-common", "thiserror 1.0.69", @@ -4095,6 +4202,15 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" +[[package]] +name = "uuid" +version = "0.8.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc5cf98d8186244414c848017f0e2676b3fcb46807f6668a97dfe67359a3c4b7" +dependencies = [ + "serde 1.0.219", +] + [[package]] name = "uuid" version = "1.18.0" diff --git a/syncstorage-postgres/src/db/db_impl.rs b/syncstorage-postgres/src/db/db_impl.rs index 53eb7bb6c1..557e31bcac 100644 --- a/syncstorage-postgres/src/db/db_impl.rs +++ b/syncstorage-postgres/src/db/db_impl.rs @@ -7,11 +7,13 @@ use diesel::{ ExpressionMethods, OptionalExtension, QueryDsl, }; use diesel_async::{AsyncConnection, RunQueryDsl, TransactionManager}; -use syncstorage_db_common::{params, results, util::SyncTimestamp, Db, Sorting}; +use syncstorage_db_common::{ + error::DbErrorIntrospect, params, results, util::SyncTimestamp, Db, Sorting, +}; use syncstorage_settings::Quota; use super::PgDb; -use crate::{bsos_query, pool::Conn, schema::bsos, DbError, DbResult}; +use crate::{bsos_query, pool::Conn, schema::bsos, schema::user_collections, DbError, DbResult}; #[async_trait(?Send)] impl Db for PgDb { @@ -51,11 +53,63 @@ impl Db for PgDb { Ok(true) } + /// Explicitly lock the matching row in the user_collections table. Read + /// locks do `SELECT ... LOCK IN SHARE MODE` and write locks do `SELECT + /// ... FOR UPDATE`. + /// + /// In theory it would be possible to use serializable transactions rather + /// than explicit locking, but our ops team have expressed concerns about + /// the efficiency of that approach at scale. async fn lock_for_read( &mut self, params: params::LockCollection, - ) -> Result { - todo!() + ) -> DbResult { + let user_id = params.user_id.legacy_id as i64; + let collection_id = self + .get_collection_id(¶ms.collection) + .await + .or_else(|e| { + if e.is_collection_not_found() { + // If the collection doesn't exist, we still want to start a + // transaction, so it will continue to not exist. + Ok(0) + } else { + Err(e) + } + })?; + + // If we already have a read or write lock then it's safe to + // use it as-is. + if self + .session + .coll_locks + .contains_key(&(user_id as u32, collection_id)) + { + return Ok(()); + } + + // Lock db. + self.begin(false).await?; + + let modified = user_collections::table + .select(user_collections::modified) + .filter(user_collections::fxa_uid.eq(user_id)) + .filter(user_collections::collection_id.eq(collection_id)) + .for_share() + .first(&mut self.conn) + .await + .optional()?; + + if let Some(modified) = modified { + let modified = SyncTimestamp::from_i64(modified)?; + self.session + .coll_modified_cache + .insert((user_id as u32, collection_id), modified); + } + self.session + .coll_locks + .insert((user_id as u32, collection_id), super::CollectionLock::Read); + Ok(()) } async fn lock_for_write( From ba3291dc52a2345cdba977855911f3861641dbe4 Mon Sep 17 00:00:00 2001 From: Taddes Date: Thu, 13 Nov 2025 20:34:32 -0500 Subject: [PATCH 05/14] update timestamp type to bigint --- .../migrations/2025-10-20-155711_create_schema/up.sql | 2 +- syncstorage-postgres/src/orm_models.rs | 4 ++-- syncstorage-postgres/src/schema.rs | 2 +- syncstorage-postgres/syncstorage_postgres_db.md | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql b/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql index 10fb6d0a5e..e43449e2d8 100644 --- a/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql +++ b/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql @@ -2,7 +2,7 @@ CREATE TABLE user_collections ( user_id BIGINT NOT NULL, collection_id INTEGER NOT NULL, - modified TIMESTAMP NOT NULL, + modified BIGINT NOT NULL, count BIGINT, total_bytes BIGINT, PRIMARY KEY (user_id, collection_id) diff --git a/syncstorage-postgres/src/orm_models.rs b/syncstorage-postgres/src/orm_models.rs index 5f757ae475..1a7c60442a 100644 --- a/syncstorage-postgres/src/orm_models.rs +++ b/syncstorage-postgres/src/orm_models.rs @@ -34,7 +34,7 @@ pub struct Bso { pub bso_id: String, pub sortindex: Option, pub payload: Vec, - pub modified: NaiveDateTime, + pub modified: i64, pub expiry: NaiveDateTime, } @@ -51,7 +51,7 @@ pub struct UserCollection { pub user_id: i64, pub collection_id: i32, - pub modified: NaiveDateTime, + pub modified: i64, pub count: Option, pub total_bytes: Option, } diff --git a/syncstorage-postgres/src/schema.rs b/syncstorage-postgres/src/schema.rs index a8434381dd..9857c9430a 100644 --- a/syncstorage-postgres/src/schema.rs +++ b/syncstorage-postgres/src/schema.rs @@ -43,7 +43,7 @@ diesel::table! { user_collections (user_id, collection_id) { user_id -> Int8, collection_id -> Int4, - modified -> Timestamp, + modified -> Bigint, count -> Nullable, total_bytes -> Nullable, } diff --git a/syncstorage-postgres/syncstorage_postgres_db.md b/syncstorage-postgres/syncstorage_postgres_db.md index 5704a96f40..e6562c2980 100644 --- a/syncstorage-postgres/syncstorage_postgres_db.md +++ b/syncstorage-postgres/syncstorage_postgres_db.md @@ -34,7 +34,7 @@ Stores actual records being synced — Basic Storage Objects. | `bso_id` | `TEXT` | Unique ID within a collection. PK (part 4) | | `sortindex` | `BIGINT` | Indicates record importance for syncing (optional) | | `payload` | `BYTEA` | Bytes payload (e.g. JSON blob) | -| `modified` | `TIMESTAMP` | Auto-assigned modification timestamp | +| `modified` | `BIGINT` | Auto-assigned modification timestamp | | `expiry` | `TIMESTAMP` | TTL as absolute expiration time (optional) | Indexes @@ -89,7 +89,7 @@ erDiagram USER_COLLECTIONS { BIGINT user_id PK INTEGER collection_id PK - TIMESTAMP modified + BIGINT modified BIGINT count BIGINT total_bytes } @@ -105,7 +105,7 @@ erDiagram TEXT bso_id PK BIGINT sortindex BYTEA payload - TIMESTAMP modified + BIGINT modified TIMESTAMP expiry } From 4e954edd241bc053c6946eb88e059c9c2fad699e Mon Sep 17 00:00:00 2001 From: Taddes Date: Mon, 17 Nov 2025 19:26:53 -0500 Subject: [PATCH 06/14] capture matching modified value for user_id and collection_id --- syncstorage-postgres/src/db/db_impl.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/syncstorage-postgres/src/db/db_impl.rs b/syncstorage-postgres/src/db/db_impl.rs index 557e31bcac..0777ba9d32 100644 --- a/syncstorage-postgres/src/db/db_impl.rs +++ b/syncstorage-postgres/src/db/db_impl.rs @@ -115,8 +115,30 @@ impl Db for PgDb { async fn lock_for_write( &mut self, params: params::LockCollection, - ) -> Result { - todo!() + ) -> DbResult { + let user_id = params.user_id.legacy_id as i64; + let collection_id = self.get_or_create_collection_id(¶ms.collection).await?; + + if let Some(CollectionLock::Read) = self + .session + .coll_locks + .get(&(user_id as u32, collection_id)) + { + return Err(DbError::internal( + "Can't escalate read-lock to write-lock".to_string(), + )); + } + + // Lock DB + self.begin(true).await?; + let modified = user_collections::table + .select(user_collections::modified) + .filter(user_collections::fxa_uid.eq(user_id)) + .filter(user_collections::collection_id.eq(collection_id)) + .for_share() + .first(&mut self.conn) + .await + .optional()?; } async fn get_collection_timestamps( From c47dd7d5b18df8e877b32a6202e5567f6d7c0bd4 Mon Sep 17 00:00:00 2001 From: Taddes Date: Mon, 17 Nov 2025 19:49:32 -0500 Subject: [PATCH 07/14] write impl --- syncstorage-postgres/src/db/db_impl.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/syncstorage-postgres/src/db/db_impl.rs b/syncstorage-postgres/src/db/db_impl.rs index 0777ba9d32..3420f12f2d 100644 --- a/syncstorage-postgres/src/db/db_impl.rs +++ b/syncstorage-postgres/src/db/db_impl.rs @@ -108,14 +108,11 @@ impl Db for PgDb { } self.session .coll_locks - .insert((user_id as u32, collection_id), super::CollectionLock::Read); + .insert((user_id as u32, collection_id), CollectionLock::Read); Ok(()) } - async fn lock_for_write( - &mut self, - params: params::LockCollection, - ) -> DbResult { + async fn lock_for_write(&mut self, params: params::LockCollection) -> DbResult<()> { let user_id = params.user_id.legacy_id as i64; let collection_id = self.get_or_create_collection_id(¶ms.collection).await?; @@ -135,10 +132,26 @@ impl Db for PgDb { .select(user_collections::modified) .filter(user_collections::fxa_uid.eq(user_id)) .filter(user_collections::collection_id.eq(collection_id)) - .for_share() + .for_update() .first(&mut self.conn) .await .optional()?; + + if let Some(modified) = modified { + let modified = SyncTimestamp::from_i64(modified)?; + // Do not allow write if it would incorrectly increment timestamp. + if modified >= self.timestamp() { + return Err(DbError::conflict()); + } + self.session + .coll_modified_cache + .insert((user_id as u32, collection_id), modified); + } + + self.session + .coll_locks + .insert((user_id as u32, collection_id), CollectionLock::Write); + Ok(()) } async fn get_collection_timestamps( From 3162e1e76b61a2910a91a923e595412d15f4bdb9 Mon Sep 17 00:00:00 2001 From: Taddes Date: Tue, 18 Nov 2025 17:45:16 -0500 Subject: [PATCH 08/14] rebase and use user_id --- syncstorage-postgres/src/db/db_impl.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/syncstorage-postgres/src/db/db_impl.rs b/syncstorage-postgres/src/db/db_impl.rs index 3420f12f2d..7e46cd068b 100644 --- a/syncstorage-postgres/src/db/db_impl.rs +++ b/syncstorage-postgres/src/db/db_impl.rs @@ -93,7 +93,7 @@ impl Db for PgDb { let modified = user_collections::table .select(user_collections::modified) - .filter(user_collections::fxa_uid.eq(user_id)) + .filter(user_collections::user_id.eq(user_id)) .filter(user_collections::collection_id.eq(collection_id)) .for_share() .first(&mut self.conn) @@ -130,7 +130,7 @@ impl Db for PgDb { self.begin(true).await?; let modified = user_collections::table .select(user_collections::modified) - .filter(user_collections::fxa_uid.eq(user_id)) + .filter(user_collections::user_id.eq(user_id)) .filter(user_collections::collection_id.eq(collection_id)) .for_update() .first(&mut self.conn) From 8dad903947d480bc7146a73313449c7a4187ef80 Mon Sep 17 00:00:00 2001 From: Taddes Date: Thu, 20 Nov 2025 18:19:57 -0500 Subject: [PATCH 09/14] rebase map --- syncstorage-postgres/src/db/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncstorage-postgres/src/db/mod.rs b/syncstorage-postgres/src/db/mod.rs index 29ba31b06e..e5ce0365d9 100644 --- a/syncstorage-postgres/src/db/mod.rs +++ b/syncstorage-postgres/src/db/mod.rs @@ -6,7 +6,7 @@ use diesel_async::RunQueryDsl; use syncserver_common::Metrics; use syncstorage_db_common::diesel::DbError; -use syncstorage_db_common::{util::SyncTimestamp, UserIdentifier}; +use syncstorage_db_common::util::SyncTimestamp; use syncstorage_settings::Quota; use super::schema::collections; From 052103979708b6cea81d22609a5dfd3906f8f489 Mon Sep 17 00:00:00 2001 From: Taddes Date: Mon, 1 Dec 2025 15:42:49 -0500 Subject: [PATCH 10/14] timestamp use --- syncstorage-postgres/src/db/db_impl.rs | 12 ++++++------ syncstorage-postgres/src/orm_models.rs | 4 ++-- syncstorage-postgres/src/schema.rs | 2 +- syncstorage-postgres/syncstorage_postgres_db.md | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/syncstorage-postgres/src/db/db_impl.rs b/syncstorage-postgres/src/db/db_impl.rs index 7e46cd068b..636dd2a600 100644 --- a/syncstorage-postgres/src/db/db_impl.rs +++ b/syncstorage-postgres/src/db/db_impl.rs @@ -91,17 +91,17 @@ impl Db for PgDb { // Lock db. self.begin(false).await?; - let modified = user_collections::table + let modified: Option = user_collections::table .select(user_collections::modified) .filter(user_collections::user_id.eq(user_id)) .filter(user_collections::collection_id.eq(collection_id)) .for_share() - .first(&mut self.conn) + .first::(&mut self.conn) .await .optional()?; if let Some(modified) = modified { - let modified = SyncTimestamp::from_i64(modified)?; + let modified = SyncTimestamp::from_i64(modified.and_utc().timestamp_millis())?; self.session .coll_modified_cache .insert((user_id as u32, collection_id), modified); @@ -128,17 +128,17 @@ impl Db for PgDb { // Lock DB self.begin(true).await?; - let modified = user_collections::table + let modified: Option = user_collections::table .select(user_collections::modified) .filter(user_collections::user_id.eq(user_id)) .filter(user_collections::collection_id.eq(collection_id)) .for_update() - .first(&mut self.conn) + .first::(&mut self.conn) .await .optional()?; if let Some(modified) = modified { - let modified = SyncTimestamp::from_i64(modified)?; + let modified = SyncTimestamp::from_i64(modified.and_utc().timestamp_millis())?; // Do not allow write if it would incorrectly increment timestamp. if modified >= self.timestamp() { return Err(DbError::conflict()); diff --git a/syncstorage-postgres/src/orm_models.rs b/syncstorage-postgres/src/orm_models.rs index 1a7c60442a..5f757ae475 100644 --- a/syncstorage-postgres/src/orm_models.rs +++ b/syncstorage-postgres/src/orm_models.rs @@ -34,7 +34,7 @@ pub struct Bso { pub bso_id: String, pub sortindex: Option, pub payload: Vec, - pub modified: i64, + pub modified: NaiveDateTime, pub expiry: NaiveDateTime, } @@ -51,7 +51,7 @@ pub struct UserCollection { pub user_id: i64, pub collection_id: i32, - pub modified: i64, + pub modified: NaiveDateTime, pub count: Option, pub total_bytes: Option, } diff --git a/syncstorage-postgres/src/schema.rs b/syncstorage-postgres/src/schema.rs index 9857c9430a..a8434381dd 100644 --- a/syncstorage-postgres/src/schema.rs +++ b/syncstorage-postgres/src/schema.rs @@ -43,7 +43,7 @@ diesel::table! { user_collections (user_id, collection_id) { user_id -> Int8, collection_id -> Int4, - modified -> Bigint, + modified -> Timestamp, count -> Nullable, total_bytes -> Nullable, } diff --git a/syncstorage-postgres/syncstorage_postgres_db.md b/syncstorage-postgres/syncstorage_postgres_db.md index e6562c2980..5704a96f40 100644 --- a/syncstorage-postgres/syncstorage_postgres_db.md +++ b/syncstorage-postgres/syncstorage_postgres_db.md @@ -34,7 +34,7 @@ Stores actual records being synced — Basic Storage Objects. | `bso_id` | `TEXT` | Unique ID within a collection. PK (part 4) | | `sortindex` | `BIGINT` | Indicates record importance for syncing (optional) | | `payload` | `BYTEA` | Bytes payload (e.g. JSON blob) | -| `modified` | `BIGINT` | Auto-assigned modification timestamp | +| `modified` | `TIMESTAMP` | Auto-assigned modification timestamp | | `expiry` | `TIMESTAMP` | TTL as absolute expiration time (optional) | Indexes @@ -89,7 +89,7 @@ erDiagram USER_COLLECTIONS { BIGINT user_id PK INTEGER collection_id PK - BIGINT modified + TIMESTAMP modified BIGINT count BIGINT total_bytes } @@ -105,7 +105,7 @@ erDiagram TEXT bso_id PK BIGINT sortindex BYTEA payload - BIGINT modified + TIMESTAMP modified TIMESTAMP expiry } From 63951c0fbd6f815e81a935bfbfd12d86d769dbb7 Mon Sep 17 00:00:00 2001 From: Taddes Date: Mon, 1 Dec 2025 15:43:57 -0500 Subject: [PATCH 11/14] migration timestamp --- .../migrations/2025-10-20-155711_create_schema/up.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql b/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql index e43449e2d8..10fb6d0a5e 100644 --- a/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql +++ b/syncstorage-postgres/migrations/2025-10-20-155711_create_schema/up.sql @@ -2,7 +2,7 @@ CREATE TABLE user_collections ( user_id BIGINT NOT NULL, collection_id INTEGER NOT NULL, - modified BIGINT NOT NULL, + modified TIMESTAMP NOT NULL, count BIGINT, total_bytes BIGINT, PRIMARY KEY (user_id, collection_id) From 19552bed762fc6b9810f24e837fef58e2516753e Mon Sep 17 00:00:00 2001 From: Taddes Date: Tue, 2 Dec 2025 15:41:32 -0500 Subject: [PATCH 12/14] revert to use UserIdentifier --- syncstorage-postgres/src/db/db_impl.rs | 28 +++++++++++++------------- syncstorage-postgres/src/db/mod.rs | 6 +++--- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/syncstorage-postgres/src/db/db_impl.rs b/syncstorage-postgres/src/db/db_impl.rs index 636dd2a600..c59ce6b943 100644 --- a/syncstorage-postgres/src/db/db_impl.rs +++ b/syncstorage-postgres/src/db/db_impl.rs @@ -64,7 +64,6 @@ impl Db for PgDb { &mut self, params: params::LockCollection, ) -> DbResult { - let user_id = params.user_id.legacy_id as i64; let collection_id = self .get_collection_id(¶ms.collection) .await @@ -83,7 +82,7 @@ impl Db for PgDb { if self .session .coll_locks - .contains_key(&(user_id as u32, collection_id)) + .contains_key(&(params.user_id.clone(), collection_id)) { return Ok(()); } @@ -93,7 +92,7 @@ impl Db for PgDb { let modified: Option = user_collections::table .select(user_collections::modified) - .filter(user_collections::user_id.eq(user_id)) + .filter(user_collections::user_id.eq(params.user_id.legacy_id as i64)) .filter(user_collections::collection_id.eq(collection_id)) .for_share() .first::(&mut self.conn) @@ -104,22 +103,22 @@ impl Db for PgDb { let modified = SyncTimestamp::from_i64(modified.and_utc().timestamp_millis())?; self.session .coll_modified_cache - .insert((user_id as u32, collection_id), modified); + .insert((params.user_id.clone(), collection_id), modified); } - self.session - .coll_locks - .insert((user_id as u32, collection_id), CollectionLock::Read); + self.session.coll_locks.insert( + (params.user_id.clone(), collection_id), + CollectionLock::Read, + ); Ok(()) } async fn lock_for_write(&mut self, params: params::LockCollection) -> DbResult<()> { - let user_id = params.user_id.legacy_id as i64; let collection_id = self.get_or_create_collection_id(¶ms.collection).await?; if let Some(CollectionLock::Read) = self .session .coll_locks - .get(&(user_id as u32, collection_id)) + .get(&(params.user_id.clone(), collection_id)) { return Err(DbError::internal( "Can't escalate read-lock to write-lock".to_string(), @@ -130,7 +129,7 @@ impl Db for PgDb { self.begin(true).await?; let modified: Option = user_collections::table .select(user_collections::modified) - .filter(user_collections::user_id.eq(user_id)) + .filter(user_collections::user_id.eq(params.user_id.legacy_id as i64)) .filter(user_collections::collection_id.eq(collection_id)) .for_update() .first::(&mut self.conn) @@ -145,12 +144,13 @@ impl Db for PgDb { } self.session .coll_modified_cache - .insert((user_id as u32, collection_id), modified); + .insert((params.user_id.clone(), collection_id), modified); } - self.session - .coll_locks - .insert((user_id as u32, collection_id), CollectionLock::Write); + self.session.coll_locks.insert( + (params.user_id.clone(), collection_id), + CollectionLock::Write, + ); Ok(()) } diff --git a/syncstorage-postgres/src/db/mod.rs b/syncstorage-postgres/src/db/mod.rs index e5ce0365d9..f61f841c7a 100644 --- a/syncstorage-postgres/src/db/mod.rs +++ b/syncstorage-postgres/src/db/mod.rs @@ -6,7 +6,7 @@ use diesel_async::RunQueryDsl; use syncserver_common::Metrics; use syncstorage_db_common::diesel::DbError; -use syncstorage_db_common::util::SyncTimestamp; +use syncstorage_db_common::{util::SyncTimestamp, UserIdentifier}; use syncstorage_settings::Quota; use super::schema::collections; @@ -53,9 +53,9 @@ struct PgDbSession { /// The "current time" on the server used for this session's operations. timestamp: SyncTimestamp, /// Cache of collection modified timestamps per (HawkIdentifier, collection_id). - coll_modified_cache: HashMap<(u32, i32), SyncTimestamp>, + coll_modified_cache: HashMap<(UserIdentifier, i32), SyncTimestamp>, /// Currently locked collections. - coll_locks: HashMap<(u32, i32), CollectionLock>, + coll_locks: HashMap<(UserIdentifier, i32), CollectionLock>, /// Whether a transaction was started (begin() called) in_transaction: bool, /// Boolean to identify if query in active transaction. From eaf9de09987f191a13f3c1bee7be4cba036e2bc6 Mon Sep 17 00:00:00 2001 From: Taddes Date: Tue, 2 Dec 2025 17:53:36 -0500 Subject: [PATCH 13/14] lock comments --- syncstorage-postgres/src/db/db_impl.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/syncstorage-postgres/src/db/db_impl.rs b/syncstorage-postgres/src/db/db_impl.rs index c59ce6b943..1bf2d962ec 100644 --- a/syncstorage-postgres/src/db/db_impl.rs +++ b/syncstorage-postgres/src/db/db_impl.rs @@ -87,7 +87,8 @@ impl Db for PgDb { return Ok(()); } - // Lock db. + // `FOR SHARE` + // Obtains shared lock, allowing multiple transactions to read rows simultaneously. self.begin(false).await?; let modified: Option = user_collections::table @@ -125,7 +126,9 @@ impl Db for PgDb { )); } - // Lock DB + // `FOR UPDATE` + // Acquires exclusive lock on select rows, prohibits other transactions from modifying + // until complete. self.begin(true).await?; let modified: Option = user_collections::table .select(user_collections::modified) From feb9210cc1265ddc0b82f4c0b561e6e6894e9857 Mon Sep 17 00:00:00 2001 From: Taddes Date: Tue, 2 Dec 2025 18:52:53 -0500 Subject: [PATCH 14/14] rebase --- syncstorage-postgres/src/db/db_impl.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/syncstorage-postgres/src/db/db_impl.rs b/syncstorage-postgres/src/db/db_impl.rs index 1bf2d962ec..dc80d8bcad 100644 --- a/syncstorage-postgres/src/db/db_impl.rs +++ b/syncstorage-postgres/src/db/db_impl.rs @@ -13,7 +13,10 @@ use syncstorage_db_common::{ use syncstorage_settings::Quota; use super::PgDb; -use crate::{bsos_query, pool::Conn, schema::bsos, schema::user_collections, DbError, DbResult}; +use crate::{ + bsos_query, db::CollectionLock, pool::Conn, schema::bsos, schema::user_collections, DbError, + DbResult, +}; #[async_trait(?Send)] impl Db for PgDb {