From b9e8aebaa4797957597fe6836a7d25081d6bfe54 Mon Sep 17 00:00:00 2001 From: Ryan Daum Date: Mon, 1 Jan 2024 12:42:56 -0500 Subject: [PATCH] Fix bug in sequence increment/max_object When importing a text dump, the value of the current max object sequence was being set one too low, causing the first created object to collide with it. --- crates/daemon/src/connections_tb.rs | 5 ++-- crates/db/src/tb_worldstate.rs | 38 ++++++++++++++++++------ crates/db/src/tuplebox/tb.rs | 31 +++++++------------ crates/db/src/tuplebox/tx/transaction.rs | 21 +++++++------ 4 files changed, 53 insertions(+), 42 deletions(-) diff --git a/crates/daemon/src/connections_tb.rs b/crates/daemon/src/connections_tb.rs index c394c4ae..d4e2ad64 100644 --- a/crates/daemon/src/connections_tb.rs +++ b/crates/daemon/src/connections_tb.rs @@ -176,7 +176,7 @@ impl ConnectionsDB for ConnectionsTb { None => { // The connection object is pulled from the sequence, then we invert it and subtract from // -4 to get the connection object, since they always grow downwards from there. - let connection_id = self.tb.clone().sequence_next(0).await; + let connection_id = self.tb.clone().increment_sequence(0).await; let connection_id: i64 = -4 - (connection_id as i64); Objid(connection_id) } @@ -413,7 +413,8 @@ impl ConnectionsDB for ConnectionsTb { .relation(RelationId(ConnectionRelation::ClientConnection as usize)) .await .seek_by_domain(client_id.as_bytes().as_sliceref()) - .await.is_ok(); + .await + .is_ok(); tx.commit().await.expect("Unable to commit transaction"); is_valid } diff --git a/crates/db/src/tb_worldstate.rs b/crates/db/src/tb_worldstate.rs index b6a71cde..49cfbb2d 100644 --- a/crates/db/src/tb_worldstate.rs +++ b/crates/db/src/tb_worldstate.rs @@ -160,7 +160,7 @@ impl DbTransaction for TupleBoxTransaction { None => { let max = self .tx - .sequence_next(WorldStateSequences::MaximumObject as usize) + .increment_sequence(WorldStateSequences::MaximumObject as usize) .await; Objid(max as i64) } @@ -202,7 +202,10 @@ impl DbTransaction for TupleBoxTransaction { // Update the maximum object number if ours is higher than the current one. This is for the // textdump case, where our numbers are coming in arbitrarily. self.tx - .update_sequence_max(WorldStateSequences::MaximumObject as usize, id.0 as u64) + .update_sequence_max( + WorldStateSequences::MaximumObject as usize, + (id.0 + 1) as u64, + ) .await; Ok(id) @@ -228,12 +231,14 @@ impl DbTransaction for TupleBoxTransaction { // Now we can remove this object from all relevant column relations // First the simple ones which are keyed on the object id. - let oid_relations = [WorldStateRelation::ObjectFlags, + let oid_relations = [ + WorldStateRelation::ObjectFlags, WorldStateRelation::ObjectName, WorldStateRelation::ObjectOwner, WorldStateRelation::ObjectParent, WorldStateRelation::ObjectLocation, - WorldStateRelation::ObjectVerbs]; + WorldStateRelation::ObjectVerbs, + ]; for rel in oid_relations.iter() { let relation = self.tx.relation((*rel).into()).await; relation @@ -533,7 +538,8 @@ impl DbTransaction for TupleBoxTransaction { Ok(Objid( self.tx .sequence_current(WorldStateSequences::MaximumObject as usize) - .await as i64, + .await as i64 + - 1, )) } @@ -557,7 +563,8 @@ impl DbTransaction for TupleBoxTransaction { .await .ok_or_else(|| WorldStateError::VerbNotFound(obj, name.clone()))?; Ok(verbdefs - .find_named(name.as_str()).first() + .find_named(name.as_str()) + .first() .ok_or(WorldStateError::VerbNotFound(obj, name))? .clone()) } @@ -853,8 +860,6 @@ impl DbTransaction for TupleBoxTransaction { Some(s) => s.as_str(), }; - - PropDef::new( p.uuid(), p.definer(), @@ -1135,7 +1140,6 @@ mod tests { relations[WorldStateRelation::ObjectParent as usize].secondary_indexed = true; relations[WorldStateRelation::ObjectLocation as usize].secondary_indexed = true; - TupleBox::new(1 << 24, 4096, None, &relations, WorldStateSequences::COUNT).await } @@ -1165,6 +1169,22 @@ mod tests { assert_eq!(tx.commit().await, Ok(CommitResult::Success)); } + #[tokio::test] + async fn test_create_object_fixed_id() { + let db = test_db().await; + let tx = TupleBoxTransaction::new(db); + // Force at 1. + let oid = tx + .create_object(Some(Objid(1)), ObjAttrs::default()) + .await + .unwrap(); + assert_eq!(oid, Objid(1)); + // Now verify the next will be 2. + let oid2 = tx.create_object(None, ObjAttrs::default()).await.unwrap(); + assert_eq!(oid2, Objid(2)); + assert_eq!(tx.commit().await, Ok(CommitResult::Success)); + } + #[tokio::test] async fn test_parent_children() { let db = test_db().await; diff --git a/crates/db/src/tuplebox/tb.rs b/crates/db/src/tuplebox/tb.rs index 1e9ca4dc..74a4a83e 100644 --- a/crates/db/src/tuplebox/tb.rs +++ b/crates/db/src/tuplebox/tb.rs @@ -131,26 +131,14 @@ impl TupleBox { } pub fn next_ts(self: Arc) -> u64 { - - self - .maximum_transaction + self.maximum_transaction .fetch_add(1, std::sync::atomic::Ordering::SeqCst) } /// Get the next value for the given sequence. pub async fn sequence_next(self: Arc, sequence_number: usize) -> u64 { let sequence = &self.sequences[sequence_number]; - loop { - let current = sequence.load(std::sync::atomic::Ordering::SeqCst); - if let Ok(n) = sequence.compare_exchange( - current, - current + 1, - std::sync::atomic::Ordering::SeqCst, - std::sync::atomic::Ordering::SeqCst, - ) { - return n; - } - } + sequence.fetch_add(1, std::sync::atomic::Ordering::SeqCst) + 1 } /// Get the current value for the given sequence. @@ -163,12 +151,15 @@ impl TupleBox { let sequence = &self.sequences[sequence_number]; loop { let current = sequence.load(std::sync::atomic::Ordering::SeqCst); - if sequence.compare_exchange( - current, - std::cmp::max(current, value), - std::sync::atomic::Ordering::SeqCst, - std::sync::atomic::Ordering::SeqCst, - ).is_ok() { + if sequence + .compare_exchange( + current, + std::cmp::max(current, value), + std::sync::atomic::Ordering::SeqCst, + std::sync::atomic::Ordering::SeqCst, + ) + .is_ok() + { return; } } diff --git a/crates/db/src/tuplebox/tx/transaction.rs b/crates/db/src/tuplebox/tx/transaction.rs index 77e80aa0..a4b1fa02 100644 --- a/crates/db/src/tuplebox/tx/transaction.rs +++ b/crates/db/src/tuplebox/tx/transaction.rs @@ -73,9 +73,10 @@ impl Transaction { next_transient_relation_id, } } - pub async fn sequence_next(&self, sequence_number: usize) -> u64 { - self.db.clone().sequence_next(sequence_number).await + pub async fn increment_sequence(&self, sequence_number: usize) -> u64 { + self.db.clone().increment_sequence(sequence_number).await } + pub async fn sequence_current(&self, sequence_number: usize) -> u64 { self.db.clone().sequence_current(sequence_number).await } @@ -342,7 +343,8 @@ impl TransientRelation { ) -> Result<(SliceRef, SliceRef), TupleError> { let tuple_id = self .domain_tuples - .get(domain.as_slice()).copied() + .get(domain.as_slice()) + .copied() .ok_or(TupleError::NotFound); tuple_id.map(|id| self.tuples[id].clone()) } @@ -358,7 +360,8 @@ impl TransientRelation { // what they're doing. let codomain_domain = self.codomain_domain.as_ref().expect("No codomain index"); let tuple_ids = codomain_domain - .get(codomain.as_slice()).cloned() + .get(codomain.as_slice()) + .cloned() .ok_or(TupleError::NotFound)?; Ok(tuple_ids .iter() @@ -370,11 +373,7 @@ impl TransientRelation { &self, f: &F, ) -> Result, TupleError> { - Ok(self - .tuples - .iter() - .filter(|t| f(t)).cloned() - .collect()) + Ok(self.tuples.iter().filter(|t| f(t)).cloned().collect()) } /// Insert a tuple into the relation. @@ -402,7 +401,8 @@ impl TransientRelation { ) -> Result<(), TupleError> { let tuple_id = self .domain_tuples - .get(domain.as_slice()).copied() + .get(domain.as_slice()) + .copied() .ok_or(TupleError::NotFound)?; if self.codomain_domain.is_some() { self.update_secondary(tuple_id, None, Some(codomain.clone())); @@ -491,7 +491,6 @@ mod tests { } async fn test_db() -> Arc { - TupleBox::new( 1 << 24, 4096,