Remove non-transactional sequence state, and disallow non-default sequence params

Cargo.toml

@@ -281,6 +281,7 @@ quick-junit = { version = "0.3.2" }
 quote = "1.0.8"
 rand08 = { package = "rand", version = "0.8" }
 rand = "0.9"
+rand_xoshiro = "0.7"
 rayon = "1.8"
 rayon-core = "1.11.0"
 regex = "1"

crates/datastore/Cargo.toml

@@ -34,6 +34,11 @@ strum.workspace = true
 thiserror.workspace = true
 thin-vec.workspace = true
 
+# For simulating sequence value skips.
+# See `fn should_simulate_sequence_reallocation` in src/locking_tx_datastore/mut_tx.rs
+rand.workspace = true
+rand_xoshiro.workspace = true
+
 [features]
 # Print a warning when doing an unindexed `iter_by_col_range` on a large table.
 unindexed_iter_by_col_range_warn = []

crates/datastore/src/error.rs

@@ -1,6 +1,6 @@
 use super::system_tables::SystemTable;
 use spacetimedb_lib::db::raw_def::{v9::RawSql, RawIndexDefV8};
-use spacetimedb_primitives::{ColId, ColList, IndexId, SequenceId, TableId, ViewId};
+use spacetimedb_primitives::{ColId, IndexId, SequenceId, TableId, ViewId};
 use spacetimedb_sats::buffer::DecodeError;
 use spacetimedb_sats::product_value::InvalidFieldError;
 use spacetimedb_sats::raw_identifier::RawNamespacedIdentifier;
@@ -116,22 +116,10 @@ pub enum SequenceError {
     Exist(String),
     #[error("Sequence `{0}`: The increment is 0, and this means the sequence can't advance.")]
     IncrementIsZero(String),
-    #[error("Sequence `{0}`: The min_value {1} must < max_value {2}.")]
-    MinMax(String, i128, i128),
-    #[error("Sequence `{0}`: The start value {1} must be >= min_value {2}.")]
-    MinStart(String, i128, i128),
-    #[error("Sequence `{0}`: The start value {1} must be <= min_value {2}.")]
-    MaxStart(String, i128, i128),
-    #[error("Sequence `{0}` failed to decode value from Sled (not a u128).")]
-    SequenceValue(String),
     #[error("Sequence ID `{0}` not found.")]
     NotFound(SequenceId),
-    #[error("Sequence applied to a non-integer field. Column `{col}` is of type {{found.to_sats()}}.")]
-    NotInteger { col: String, found: AlgebraicType },
-    #[error("Sequence ID `{0}` still had no values left after allocation.")]
-    UnableToAllocate(SequenceId),
-    #[error("Autoinc constraint on table {0:?} spans more than one column: {1:?}")]
-    MultiColumnAutoInc(TableId, ColList),
+    #[error("Incrementing sequence with previous value {0} would result in integer overflow")]
+    Overflow(i128),
 }
 
 impl From<InvalidFieldError> for DatastoreError {

crates/datastore/src/locking_tx_datastore/committed_state.rs

@@ -1,7 +1,6 @@
 use super::{
     datastore::Result,
     delete_table::DeleteTable,
-    sequence::{Sequence, SequencesState},
     state_view::StateView,
     tx_state::{IndexIdMap, PendingSchemaChange, TxState},
     IterByColEqTx,
@@ -36,13 +35,15 @@ use crate::{
 };
 use anyhow::anyhow;
 use core::{convert::Infallible, ops::RangeBounds};
+use rand::SeedableRng;
+use rand_xoshiro::Xoshiro128PlusPlus;
 use spacetimedb_data_structures::map::{HashMap, HashSet, IntMap, IntSet};
 use spacetimedb_durability::TxOffset;
 use spacetimedb_lib::{db::auth::StTableType, Identity};
 use spacetimedb_primitives::{ColList, IndexId, TableId};
 use spacetimedb_sats::memory_usage::MemoryUsage;
 use spacetimedb_sats::{AlgebraicValue, ProductValue};
-use spacetimedb_schema::schema::TableSchema;
+use spacetimedb_schema::schema::{SequenceSchema, TableSchema};
 use spacetimedb_table::{
     blob_store::{BlobStore, HashMapBlobStore},
     indexes::{RowPointer, SquashedOffset},
@@ -95,6 +96,17 @@ pub struct CommittedState {
     ///     - system tables: `st_view_sub`, `st_view_arg`
     ///     - Tables which back views.
     pub(super) ephemeral_tables: EphemeralTables,
+
+    /// RNG source for deciding when advancing a sequence should simulate a reallocation.
+    ///
+    /// We don't want users to depend on sequence values being strictly sequential,
+    /// as we have in the past and may in the future used optimizations that would cause values to be skipped.
+    /// To prevent this, [`get_next_sequence_value`](super::mut_tx::get_next_sequence_value) occasionally simulates a skip ahead.
+    ///
+    /// We use an explicit PRNG here rather than reading from the thread RNG because we'd like our tests to be deterministic.
+    ///
+    /// We chose Xoshiro128++ because it is fast and small, and we do not need a cryptographically secure PRNG for this purpose.
+    pub(super) sequence_advance_simulate_reallocation_rng: Xoshiro128PlusPlus,
 }
 
 impl CommittedState {
@@ -130,6 +142,9 @@ impl MemoryUsage for CommittedState {
             read_sets,
             view_instances,
             ephemeral_tables,
+            // Don't include the PRNG; it doesn't live on or use the heap,
+            // and it's easier to just ignore it here than to write a trait impl that returns zero.
+            sequence_advance_simulate_reallocation_rng: _,
         } = self;
         // NOTE(centril): We do not want to include the heap usage of `page_pool` as it's a shared resource.
         next_tx_offset.heap_usage()
@@ -211,6 +226,16 @@ impl CommittedState {
             page_pool,
             datastore_page_bytes: 0,
             ephemeral_tables: <_>::default(),
+            sequence_advance_simulate_reallocation_rng: {
+                #[cfg(test)]
+                {
+                    Xoshiro128PlusPlus::seed_from_u64(0)
+                }
+                #[cfg(not(test))]
+                {
+                    Xoshiro128PlusPlus::from_rng(&mut rand::rng())
+                }
+            },
         }
     }
 
@@ -367,9 +392,9 @@ impl CommittedState {
                 sequence_name: seq.sequence_name.clone(),
                 table_id: seq.table_id,
                 col_pos: seq.col_pos,
-                increment: seq.increment,
-                min_value: seq.min_value,
-                max_value: seq.max_value,
+                increment: SequenceSchema::INCREMENT,
+                min_value: SequenceSchema::MIN_VALUE,
+                max_value: SequenceSchema::MAX_VALUE,
                 start: seq.start,
                 // In practice, this means we will actually start at start - 1, since `allocated`
                 // overrides start, but we keep these fields set this way to match databases
@@ -420,31 +445,6 @@ impl CommittedState {
         Ok(())
     }
 
-    /// Builds the in-memory state of sequences from `st_sequence` system table.
-    /// The tables store the lasted allocated value, which tells us where to start generating.
-    pub(super) fn build_sequence_state(&mut self) -> Result<SequencesState> {
-        let mut sequence_state = SequencesState::default();
-        let st_sequences = self.tables.get(&ST_SEQUENCE_ID).unwrap();
-        for row_ref in st_sequences.scan_rows(&self.blob_store) {
-            let sequence = StSequenceRow::try_from(row_ref)?;
-            let seq = Sequence::new(sequence.clone().into(), Some(sequence.allocated));
-
-            // Clobber any existing in-memory `Sequence`.
-            // Such a value may exist because, when replaying without a snapshot,
-            // `build_sequence_state` is called twice:
-            // once when bootstrapping the empty datastore,
-            // and then again after replaying the commitlog.
-            // At this latter time, `sequence_state.get(seq.id())` for the system table sequences
-            // will return a sequence with incorrect `allocated`,
-            // as it will reflect the state after initializing the system tables,
-            // but before creating any user tables.
-            // The `sequence` we read out of `row_ref` above, and used to construct `seq`,
-            // will correctly reflect the state after creating user tables.
-            sequence_state.insert(seq);
-        }
-        Ok(sequence_state)
-    }
-
     /// Returns an iterator doing a full table scan on `table_id`.
     pub(super) fn table_scan<'a>(&'a self, table_id: TableId) -> Option<TableScanIter<'a>> {
         Some(self.get_table(table_id)?.scan_rows(&self.blob_store))
@@ -769,20 +769,16 @@ impl CommittedState {
     }
 
     /// Rolls back the changes immediately made to the committed state during a transaction.
-    pub(super) fn rollback(&mut self, seq_state: &mut SequencesState, tx_state: TxState) -> TxOffset {
+    pub(super) fn rollback(&mut self, tx_state: TxState) -> TxOffset {
         // Roll back the changes in the reverse order in which they were made
         // so that e.g., the last change is undone first.
         for change in tx_state.pending_schema_changes.into_iter().rev() {
-            self.rollback_pending_schema_change(seq_state, change);
+            self.rollback_pending_schema_change(change);
         }
         self.next_tx_offset.saturating_sub(1)
     }
 
-    fn rollback_pending_schema_change(
-        &mut self,
-        seq_state: &mut SequencesState,
-        change: PendingSchemaChange,
-    ) -> Option<()> {
+    fn rollback_pending_schema_change(&mut self, change: PendingSchemaChange) -> Option<()> {
         use PendingSchemaChange::*;
         match change {
             // An index was removed. Add it back.
@@ -923,16 +919,14 @@ impl CommittedState {
                 }
             }
             // A sequence was removed. Add it back.
-            SequenceRemoved(table_id, seq, schema) => {
+            SequenceRemoved(table_id, schema) => {
                 let table = self.tables.get_mut(&table_id)?;
                 table.with_mut_schema(|s| s.update_sequence(schema));
-                seq_state.insert(seq);
             }
             // A sequence was added. Remove it.
             SequenceAdded(table_id, sequence_id) => {
                 let table = self.tables.get_mut(&table_id)?;
                 table.with_mut_schema(|s| s.remove_sequence(sequence_id));
-                seq_state.remove(sequence_id);
             }
         }