Skip to content

Commit

Permalink
Fix bug in sequence increment/max_object
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rdaum committed Jan 1, 2024
1 parent 559ae3e commit 1b47930
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 44 deletions.
5 changes: 3 additions & 2 deletions crates/daemon/src/connections_tb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down
38 changes: 29 additions & 9 deletions crates/db/src/tb_worldstate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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,
))
}

Expand All @@ -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())
}
Expand Down Expand Up @@ -853,8 +860,6 @@ impl DbTransaction for TupleBoxTransaction {
Some(s) => s.as_str(),
};



PropDef::new(
p.uuid(),
p.definer(),
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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;
Expand Down
35 changes: 13 additions & 22 deletions crates/db/src/tuplebox/tb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,26 +131,14 @@ impl TupleBox {
}

pub fn next_ts(self: Arc<Self>) -> 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<Self>, sequence_number: usize) -> u64 {
/// Increment this sequence and return its previous value.
pub async fn increment_sequence(self: Arc<Self>, 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)
}

/// Get the current value for the given sequence.
Expand All @@ -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;
}
}
Expand Down
21 changes: 10 additions & 11 deletions crates/db/src/tuplebox/tx/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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())
}
Expand All @@ -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()
Expand All @@ -370,11 +373,7 @@ impl TransientRelation {
&self,
f: &F,
) -> Result<Vec<(SliceRef, SliceRef)>, 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.
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -491,7 +491,6 @@ mod tests {
}

async fn test_db() -> Arc<TupleBox> {

TupleBox::new(
1 << 24,
4096,
Expand Down

0 comments on commit 1b47930

Please sign in to comment.