Remove set_read_only from Persistence trait, and make it independent of the lease (#42108)

GitOrigin-RevId: 9e91aa25c3c38fba3f973aa820f6e1b64f685120

Author: goffrie

crates/common/src/persistence.rs

@@ -214,8 +214,6 @@ pub trait Persistence: Sync + Send + 'static {
         conflict_strategy: ConflictStrategy,
     ) -> anyhow::Result<()>;
 
-    async fn set_read_only(&self, read_only: bool) -> anyhow::Result<()>;
-
     /// Writes global key-value data for the whole persistence.
     /// This is expected to be small data that does not make sense in a
     /// versioned or transaction context. See `PersistenceGlobalKey`.

crates/common/src/testing/persistence_test_suite.rs

@@ -77,7 +77,7 @@ use crate::{
 
 #[macro_export]
 macro_rules! run_persistence_test_suite {
-    ($db:ident, $create_db:expr, $create_persistence:expr, $create_persistence_read_only:expr) => {
+    ($db:ident, $create_db:expr, $create_persistence:expr) => {
         #[tokio::test]
         async fn test_persistence_write_and_load() -> anyhow::Result<()> {
             let $db = $create_db;
@@ -192,16 +192,6 @@ macro_rules! run_persistence_test_suite {
             .await
         }
 
-        #[tokio::test]
-        async fn test_persistence_set_read_only() -> anyhow::Result<()> {
-            let $db = $create_db;
-            persistence_test_suite::set_read_only(
-                || async { Ok($create_persistence) },
-                || async { Ok($create_persistence_read_only) },
-            )
-            .await
-        }
-
         #[tokio::test]
         async fn test_persistence_global() -> anyhow::Result<()> {
             let $db = $create_db;
@@ -1485,73 +1475,6 @@ where
     Ok(())
 }
 
-pub async fn set_read_only<F, Fut, F1, Fut1, P: Persistence>(
-    mut make_p: F,
-    mut make_p_read_only: F1,
-) -> anyhow::Result<()>
-where
-    F: FnMut() -> Fut,
-    Fut: Future<Output = anyhow::Result<P>>,
-    F1: FnMut() -> Fut1,
-    Fut1: Future<Output = anyhow::Result<P>>,
-{
-    // Initially not read-only.
-    let p_backend1 = make_p().await?;
-    let table: TableName = str::parse("table")?;
-    let mut id_generator = TestIdGenerator::new();
-    let doc_id = id_generator.user_generate(&table);
-
-    let doc = ResolvedDocument::new(doc_id, CreationTime::ONE, ConvexObject::empty())?;
-
-    p_backend1
-        .write(
-            &[DocumentLogEntry {
-                ts: Timestamp::must(0),
-                id: doc.id_with_table_id(),
-                value: Some(doc.clone()),
-                prev_ts: None,
-            }],
-            &[],
-            ConflictStrategy::Error,
-        )
-        .await?;
-    // Release the lease.
-    drop(p_backend1);
-
-    let p_migration = make_p().await?;
-    p_migration.set_read_only(true).await?;
-
-    let result = make_p().await;
-    assert!(result.is_err());
-
-    drop(p_migration);
-
-    // Try to acquire lease should fail because it's read-only.
-    let result = make_p().await;
-    assert!(result.is_err());
-
-    let p_cleanup = make_p_read_only().await?;
-    p_cleanup.set_read_only(false).await?;
-    drop(p_cleanup);
-
-    // Now it's no longer read-only.
-    let p_backend2 = make_p().await?;
-    p_backend2
-        .write(
-            &[(DocumentLogEntry {
-                ts: Timestamp::must(1),
-                id: doc.id_with_table_id(),
-                value: Some(doc.clone()),
-                prev_ts: None,
-            })],
-            &[],
-            ConflictStrategy::Error,
-        )
-        .await?;
-
-    Ok(())
-}
-
 pub async fn persistence_global<P: Persistence>(p: Arc<P>) -> anyhow::Result<()> {
     let key = PersistenceGlobalKey::RetentionMinSnapshotTimestamp;
     p.write_persistence_global(key, json!(5)).await?;

crates/common/src/testing/test_persistence.rs

@@ -68,17 +68,15 @@ impl TestPersistence {
         config_test();
         let inner = Inner {
             is_fresh: true,
-            is_read_only: false,
             log: BTreeMap::new(),
             index: BTreeMap::new(),
             persistence_globals: BTreeMap::new(),
         };
-        Self::new_inner(Arc::new(Mutex::new(inner)), false).unwrap()
+        Self::new_inner(Arc::new(Mutex::new(inner))).unwrap()
     }
 
     /// Pass in an Inner to store state across TestPersistence instances.
-    fn new_inner(inner: Arc<Mutex<Inner>>, allow_read_only: bool) -> anyhow::Result<Self> {
-        anyhow::ensure!(allow_read_only || !inner.lock().is_read_only);
+    fn new_inner(inner: Arc<Mutex<Inner>>) -> anyhow::Result<Self> {
         Ok(Self { inner })
     }
 }
@@ -139,11 +137,6 @@ impl Persistence for TestPersistence {
         Ok(())
     }
 
-    async fn set_read_only(&self, read_only: bool) -> anyhow::Result<()> {
-        self.inner.lock().is_read_only = read_only;
-        Ok(())
-    }
-
     async fn write_persistence_global(
         &self,
         key: PersistenceGlobalKey,
@@ -421,7 +414,6 @@ impl PersistenceReader for TestPersistence {
 
 struct Inner {
     is_fresh: bool,
-    is_read_only: bool,
     log: BTreeMap<(Timestamp, InternalDocumentId), (Option<ResolvedDocument>, Option<Timestamp>)>,
     index: BTreeMap<IndexId, BTreeMap<(IndexKeyBytes, Timestamp), Option<InternalDocumentId>>>,
     persistence_globals: BTreeMap<PersistenceGlobalKey, JsonValue>,
@@ -450,11 +442,9 @@ run_persistence_test_suite!(
     db,
     Arc::new(Mutex::new(Inner {
         is_fresh: true,
-        is_read_only: false,
         log: BTreeMap::new(),
         index: BTreeMap::new(),
         persistence_globals: BTreeMap::new(),
     })),
-    TestPersistence::new_inner(db.clone(), false)?,
-    TestPersistence::new_inner(db.clone(), true)?
+    TestPersistence::new_inner(db.clone())?
 );