4126 bug internal database error ts is less than or equal to the latest ts (#4311)

core/src/cf/gc.rs

@@ -57,11 +57,8 @@ pub async fn gc_ns(tx: &Transaction, ts: u64, ns: &str) -> Result<(), Error> {
 		// Calculate the watermark expiry window
 		let watermark_ts = ts - cf_expiry;
 		// Calculate the watermark versionstamp
-		let watermark_vs = tx
+		let watermark_vs =
-			.lock()
+			tx.lock().await.get_versionstamp_from_timestamp(watermark_ts, ns, &db.name).await?;
-			.await
-			.get_versionstamp_from_timestamp(watermark_ts, ns, &db.name, true)
-			.await?;
 		// If a versionstamp exists, then garbage collect
 		if let Some(watermark_vs) = watermark_vs {
 			gc_range(tx, ns, &db.name, watermark_vs).await?;

core/src/cf/reader.rs

@@ -28,7 +28,7 @@ pub async fn read(
 		ShowSince::Versionstamp(x) => change::prefix_ts(ns, db, vs::u64_to_versionstamp(x)),
 		ShowSince::Timestamp(x) => {
 			let ts = x.0.timestamp() as u64;
-			let vs = tx.lock().await.get_versionstamp_from_timestamp(ts, ns, db, true).await?;
+			let vs = tx.lock().await.get_versionstamp_from_timestamp(ts, ns, db).await?;
 			match vs {
 				Some(vs) => change::prefix_ts(ns, db, vs),
 				None => {

core/src/cf/writer.rs

@@ -401,8 +401,8 @@ mod tests {
 		.await;
 		ds.tick_at(10).await.unwrap();
 		let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
-		let vs1 = tx.get_versionstamp_from_timestamp(5, NS, DB, false).await.unwrap().unwrap();
+		let vs1 = tx.get_versionstamp_from_timestamp(5, NS, DB).await.unwrap().unwrap();
-		let vs2 = tx.get_versionstamp_from_timestamp(10, NS, DB, false).await.unwrap().unwrap();
+		let vs2 = tx.get_versionstamp_from_timestamp(10, NS, DB).await.unwrap().unwrap();
 		tx.cancel().await.unwrap();
 		let _id2 = record_change_feed_entry(
 			ds.transaction(Write, Optimistic).await.unwrap(),

core/src/kvs/ds.rs

@@ -681,6 +681,8 @@ impl Datastore {
 			for db in dbs {
 				let db = db.name.as_str();
 				// TODO(SUR-341): This is incorrect, it's a [ns,db] to vs pair
+				// It's safe for now, as it is unused but either the signature must change
+				// to include {(ns, db): (ts, vs)} mapping, or we don't return it
 				vs = Some(tx.lock().await.set_timestamp_for_versionstamp(ts, ns, db).await?);
 			}
 		}

core/src/kvs/tests/helper.rs

@@ -1,4 +1,5 @@
 use crate::dbs::node::Timestamp;
+use crate::dbs::Session;
 use crate::kvs::clock::{FakeClock, SizedClock};
 use crate::kvs::tests::{ClockType, Kvs};
 use crate::kvs::Datastore;

core/src/kvs/tests/timestamp_to_versionstamp.rs

@@ -22,7 +22,7 @@ async fn timestamp_to_versionstamp() {
 	tx.commit().await.unwrap();
 	// Get the versionstamp for timestamp 0
 	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
-	let vs1 = tx.get_versionstamp_from_timestamp(0, "myns", "mydb", true).await.unwrap().unwrap();
+	let vs1 = tx.get_versionstamp_from_timestamp(0, "myns", "mydb").await.unwrap().unwrap();
 	tx.commit().await.unwrap();
 	// Give the current versionstamp a timestamp of 1
 	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
@@ -30,7 +30,7 @@ async fn timestamp_to_versionstamp() {
 	tx.commit().await.unwrap();
 	// Get the versionstamp for timestamp 1
 	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
-	let vs2 = tx.get_versionstamp_from_timestamp(1, "myns", "mydb", true).await.unwrap().unwrap();
+	let vs2 = tx.get_versionstamp_from_timestamp(1, "myns", "mydb").await.unwrap().unwrap();
 	tx.commit().await.unwrap();
 	// Give the current versionstamp a timestamp of 2
 	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
@@ -38,8 +38,64 @@ async fn timestamp_to_versionstamp() {
 	tx.commit().await.unwrap();
 	// Get the versionstamp for timestamp 2
 	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
-	let vs3 = tx.get_versionstamp_from_timestamp(2, "myns", "mydb", true).await.unwrap().unwrap();
+	let vs3 = tx.get_versionstamp_from_timestamp(2, "myns", "mydb").await.unwrap().unwrap();
 	tx.commit().await.unwrap();
 	assert!(vs1 < vs2);
 	assert!(vs2 < vs3);
 }
+
+#[tokio::test]
+#[serial]
+async fn writing_ts_again_results_in_following_ts() {
+	// Create a new datastore
+	let node_id = Uuid::parse_str("A905CA25-56ED-49FB-B759-696AEA87C342").unwrap();
+	let clock = Arc::new(SizedClock::Fake(FakeClock::new(Timestamp::default())));
+	let (ds, _) = new_ds(node_id, clock).await;
+
+	// Declare ns/db
+	ds.execute("USE NS myns; USE DB mydb; CREATE record", &Session::owner(), None).await.unwrap();
+
+	// Give the current versionstamp a timestamp of 0
+	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
+	tx.set_timestamp_for_versionstamp(0, "myns", "mydb").await.unwrap();
+	tx.commit().await.unwrap();
+
+	// Get the versionstamp for timestamp 0
+	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
+	let vs1 = tx.get_versionstamp_from_timestamp(0, "myns", "mydb").await.unwrap().unwrap();
+	tx.commit().await.unwrap();
+
+	// Give the current versionstamp a timestamp of 1
+	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
+	tx.set_timestamp_for_versionstamp(1, "myns", "mydb").await.unwrap();
+	tx.commit().await.unwrap();
+
+	// Get the versionstamp for timestamp 1
+	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
+	let vs2 = tx.get_versionstamp_from_timestamp(1, "myns", "mydb").await.unwrap().unwrap();
+	tx.commit().await.unwrap();
+
+	assert!(vs1 < vs2);
+
+	// Scan range
+	let start = crate::key::database::ts::new("myns", "mydb", 0);
+	let end = crate::key::database::ts::new("myns", "mydb", u64::MAX);
+	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
+	let scanned = tx.scan(start.clone()..end.clone(), u32::MAX, None).await.unwrap();
+	tx.commit().await.unwrap();
+	assert_eq!(scanned.len(), 2);
+	assert_eq!(scanned[0].0, crate::key::database::ts::new("myns", "mydb", 0).encode().unwrap());
+	assert_eq!(scanned[1].0, crate::key::database::ts::new("myns", "mydb", 1).encode().unwrap());
+
+	// Repeating tick
+	ds.tick_at(1).await.unwrap();
+
+	// Validate
+	let mut tx = ds.transaction(Write, Optimistic).await.unwrap().inner();
+	let scanned = tx.scan(start..end, u32::MAX, None).await.unwrap();
+	tx.commit().await.unwrap();
+	assert_eq!(scanned.len(), 3);
+	assert_eq!(scanned[0].0, crate::key::database::ts::new("myns", "mydb", 0).encode().unwrap());
+	assert_eq!(scanned[1].0, crate::key::database::ts::new("myns", "mydb", 1).encode().unwrap());
+	assert_eq!(scanned[2].0, crate::key::database::ts::new("myns", "mydb", 2).encode().unwrap());
+}

core/src/kvs/tr.rs

@@ -598,7 +598,7 @@ impl Transactor {
 
 		// Ensure there are no keys after the ts_key
 		// Otherwise we can go back in time!
-		let ts_key = crate::key::database::ts::new(ns, db, ts);
+		let mut ts_key = crate::key::database::ts::new(ns, db, ts);
 		let begin = ts_key.encode()?;
 		let end = crate::key::database::ts::suffix(ns, db);
 		let ts_pairs: Vec<(Vec<u8>, Vec<u8>)> = self.getr(begin..end, None).await?;
@@ -615,9 +615,8 @@ impl Transactor {
 			let k = crate::key::database::ts::Ts::decode(k)?;
 			let latest_ts = k.ts;
 			if latest_ts >= ts {
-				return Err(Error::Internal(
+				warn!("ts {ts} is less than the latest ts {latest_ts}");
-					"ts is less than or equal to the latest ts".to_string(),
+				ts_key = crate::key::database::ts::new(ns, db, latest_ts + 1);
-				));
 			}
 		}
 		self.set(ts_key, vst, None).await?;
@@ -629,7 +628,6 @@ impl Transactor {
 		ts: u64,
 		ns: &str,
 		db: &str,
-		_lock: bool,
 	) -> Result<Option<Versionstamp>, Error> {
 		let start = crate::key::database::ts::prefix(ns, db);
 		let ts_key = crate::key::database::ts::new(ns, db, ts + 1);