fix(log) log unreachable nodes

crates/pdfs-core/src/db/migrations.rs

@@ -9,7 +9,7 @@ use super::Db;
 use crate::Result;
 
 /// Current schema version. Bump on every forward migration added below.
-pub(super) const SCHEMA_VERSION: i64 = 26;
+pub(super) const SCHEMA_VERSION: i64 = 27;
 
 impl Db {
     pub(super) fn migrate(&self) -> Result<()> {
@@ -182,6 +182,25 @@ impl Db {
                 tx.execute_batch(MIGRATION_V26)?;
             }
         }
+        if current < 27 {
+            // Same guards as V26, for the same two reasons: the table may not
+            // exist in an old fixture, and a fixture rewound from the current
+            // schema already has the column.
+            let has_column: bool = tx.query_row(
+                "SELECT COUNT(*) FROM pragma_table_info('pending_op') \
+                 WHERE name = 'access_deferred_since'",
+                [],
+                |row| row.get::<_, i64>(0),
+            )? > 0;
+            let has_ops: bool = tx.query_row(
+                "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'pending_op'",
+                [],
+                |row| row.get::<_, i64>(0),
+            )? > 0;
+            if has_ops && !has_column {
+                tx.execute_batch(MIGRATION_V27)?;
+            }
+        }
         tx.execute(
             "INSERT INTO sync_state (key, value) VALUES ('schema_version', ?1)
              ON CONFLICT(key) DO UPDATE SET value = excluded.value",
@@ -769,3 +788,23 @@ ALTER TABLE pending_op ADD COLUMN claimed_at INTEGER NOT NULL DEFAULT 0;
 CREATE INDEX IF NOT EXISTS idx_pending_op_claimed
     ON pending_op(uid) WHERE claimed_at <> 0;
 ";
+
+/// Schema v27: when an op was first deferred by the local access check.
+///
+/// An access deferral consumes no attempt and records no error — the op has not
+/// failed remotely, it has not been sent at all. That is right for a permission
+/// change the user is about to undo, and wrong for a condition that never
+/// clears: the row is re-deferred every
+/// [`DRAIN_ACCESS_RECHECK`](../../../pdfs_fuse/drain/constant.DRAIN_ACCESS_RECHECK.html)
+/// forever while `attempts`, `last_error`, `failing` and the logs all stay
+/// empty, so a queue that can never drain is indistinguishable from a busy one.
+/// Found in the field as 28 revision ops holding 18 GiB of accepted writes for
+/// 30 days, none of them visible anywhere.
+///
+/// `0` means "not currently access-deferred"; anything else is the ms at which
+/// the first consecutive deferral happened, which is what
+/// [`Db::defer_op_for_access`](super::Db::defer_op_for_access) measures the
+/// deferral against.
+const MIGRATION_V27: &str = "
+ALTER TABLE pending_op ADD COLUMN access_deferred_since INTEGER NOT NULL DEFAULT 0;
+";

crates/pdfs-core/src/db/ops.rs

@@ -721,11 +721,16 @@ impl Db {
 
     /// Record a failed attempt and when to next try. Leaves the row in place —
     /// the staged bytes are still the only copy of the user's write.
+    ///
+    /// Clears any access-deferral window: whatever the op was waiting for has
+    /// now been reported, so the next uncleared deferral starts its own window
+    /// rather than escalating again on the following recheck.
     pub fn record_op_failure(&self, id: i64, error: &str, next_attempt_at: i64) -> Result<()> {
         let conn = self.conn.lock();
         conn.execute(
             "UPDATE pending_op
-             SET attempts = attempts + 1, last_error = ?2, next_attempt_at = ?3
+             SET attempts = attempts + 1, last_error = ?2, next_attempt_at = ?3,
+                 access_deferred_since = 0
              WHERE id = ?1",
             params![id, error, next_attempt_at],
         )?;
@@ -751,6 +756,60 @@ impl Db {
         Ok(())
     }
 
+    /// Defer an op the local access check refused, and report how long it has
+    /// been refused for.
+    ///
+    /// Identical to [`defer_op_without_attempt`](Self::defer_op_without_attempt)
+    /// on the retry clock, plus the one thing that was missing: the first
+    /// deferral of a run stamps `access_deferred_since`, and every deferral
+    /// returns that stamp. A caller can therefore tell a deferral that is one
+    /// recheck old from one that has been repeating unchanged for days, which is
+    /// the difference between a permission the user is mid-way through changing
+    /// and a queue that will never drain.
+    ///
+    /// Returns the ms at which the current run of deferrals began — `now` on the
+    /// first one.
+    pub fn defer_op_for_access(&self, id: i64, now: i64, next_attempt_at: i64) -> Result<i64> {
+        let conn = self.conn.lock();
+        conn.execute(
+            "UPDATE pending_op
+             SET next_attempt_at = CASE
+                 WHEN next_attempt_at >= ?4 THEN next_attempt_at
+                 ELSE ?3
+             END,
+             access_deferred_since = CASE
+                 WHEN access_deferred_since = 0 THEN ?2
+                 ELSE access_deferred_since
+             END
+             WHERE id = ?1",
+            params![id, now, next_attempt_at, PARK_UNTIL],
+        )?;
+        let since = conn
+            .query_row(
+                "SELECT access_deferred_since FROM pending_op WHERE id = ?1",
+                params![id],
+                |r| r.get::<_, i64>(0),
+            )
+            .optional()?;
+        // A row that drained out from under this call leaves nothing to measure;
+        // reporting `now` says "this run just started", which is the reading that
+        // never escalates something that no longer exists.
+        Ok(since.filter(|since| *since != 0).unwrap_or(now))
+    }
+
+    /// Forget an op's access-deferral window, after an attempt the access check
+    /// let through. The op is no longer blocked, so a later deferral is the
+    /// start of a new run rather than the continuation of an old one.
+    pub fn clear_op_access_deferral(&self, id: i64) -> Result<()> {
+        let conn = self.conn.lock();
+        conn.execute(
+            "UPDATE pending_op SET access_deferred_since = 0
+             WHERE id = ?1 AND access_deferred_since <> 0",
+            params![id],
+        )?;
+        Ok(())
+    }
+
     /// Every staged blob the queue still refers to.
     ///
     /// The startup staging reconcile subtracts this from what is actually in

crates/pdfs-core/src/db/tests.rs

@@ -3491,6 +3491,60 @@ fn migration_v25_backfills_paths_for_an_existing_tree() {
     let _ = std::fs::remove_file(format!("{}.lock", path.display()));
 }
 
+/// A queue carried over from V26 keeps its rows, and every one of them starts
+/// out not access-deferred — a migration must not invent a deferral window that
+/// would report an ordinary queued upload as blocked.
+#[test]
+fn migration_v27_adds_the_access_deferral_column_to_a_v26_queue() {
+    let path = std::env::temp_dir().join(format!(
+        "pdfs-db-v26-fixture-{}-{}.db",
+        std::process::id(),
+        std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap()
+            .as_nanos()
+    ));
+    {
+        let db = Db::open(&path).unwrap();
+        db.enqueue_op(&crate::db::PendingOp {
+            id: 0,
+            kind: crate::db::OP_REVISION.to_string(),
+            uid: "v~l".to_string(),
+            parent_uid: None,
+            name: None,
+            blob_path: Some("/tmp/blob".to_string()),
+            meta_json: Some("{}".to_string()),
+            created_at: 1,
+            attempts: 0,
+            last_error: None,
+            next_attempt_at: 0,
+        })
+        .unwrap();
+    }
+    {
+        // Put the file back in the state a released V26 database was in.
+        let conn = rusqlite::Connection::open(&path).unwrap();
+        conn.execute_batch(
+            "ALTER TABLE pending_op DROP COLUMN access_deferred_since;
+             UPDATE sync_state SET value = '26' WHERE key = 'schema_version';",
+        )
+        .unwrap();
+    }
+
+    let db = Db::open(&path).unwrap();
+    let ops = db.pending_ops().unwrap();
+    assert_eq!(ops.len(), 1, "the migration keeps the queued write");
+    let now = 5_000i64;
+    assert_eq!(
+        db.defer_op_for_access(ops[0].id, now, now + 5_000).unwrap(),
+        now,
+        "a carried-over op is not already deferred"
+    );
+
+    let _ = std::fs::remove_file(&path);
+    let _ = std::fs::remove_file(format!("{}.lock", path.display()));
+}
+
 // --- read-only connection pool (DB2) ---------------------------------------
 
 /// A read must not queue behind whatever happens to be writing.