only persist documents to S3 when dirty (#416)

Fix an issue where y-sweet was persisting documents to S3 every ~10
seconds even when no changes had occurred.

From what I understand, the persistence worker wakes up both on the
dirty signal and also on a `checkpoint_freq` interval and was always
calling persist, regardless of the dirtiness, which would write it to S3
regardless of whether any changes had been made.

The fix adds a check to only persist when the document is actually
dirty.

I'm still a little bit unclear behind the reasoning for the logic of
signaling the worker both on a dirty callback and on a timer.

So I wonder if we instead just start the worker once, wake every 10
seconds, check if dirty, persist if needed.

Author: lxe

crates/y-sweet-core/src/sync_kv.rs

@@ -56,7 +56,17 @@ impl SyncKv {
         }
     }
 
+    pub fn is_dirty(&self) -> bool {
+        self.dirty.load(Ordering::Relaxed)
+    }
+
     pub async fn persist(&self) -> Result<(), Box<dyn std::error::Error>> {
+        // Only persist if actually dirty
+        if !self.is_dirty() {
+            tracing::info!("Not persisting, no changes detected");
+            return Ok(());
+        }
+
         if let Some(store) = &self.store {
             let snapshot = {
                 let data = self.data.lock().unwrap();
@@ -299,4 +309,29 @@ mod test {
             assert_eq!(sync_kv.get(b"foo"), Some(b"bar".to_vec()));
         }
     }
+
+    #[tokio::test]
+    async fn only_persists_when_dirty() {
+        let store = MemoryStore::default();
+        let sync_kv = SyncKv::new(Some(Arc::new(Box::new(store.clone()))), "foo", || ())
+            .await
+            .unwrap();
+
+        assert!(!sync_kv.is_dirty());
+
+        sync_kv.set(b"foo", b"bar");
+        assert!(sync_kv.is_dirty());
+
+        sync_kv.persist().await.unwrap();
+        assert!(!sync_kv.is_dirty());
+        assert_eq!(store.data.len(), 1);
+
+        store.data.clear();
+
+        assert!(!sync_kv.is_dirty());
+        sync_kv.persist().await.unwrap();
+
+        // Should not persist when not dirty
+        assert!(store.data.is_empty());
+    }
 }