[Feature]: Path locking and selective crash recovery for storage operations

[qin-ctx]

RFC Discussion: #115

Problem Statement

OpenViking's core write operations (rm, mv, add_resource, session.commit) coordinate across multiple subsystems — VikingFS, VectorDB, and QueueManager. Without coordination:

• Concurrent operations on the same path can corrupt data
• session.commit() crashes mid-way through memory extraction → extracted memories lost permanently
• rm() / mv() partial failures leave inconsistent state between FS and VectorDB

Solution

Implement path-level locking for concurrent access coordination and selective crash recovery (redo-log) for session memory extraction — the one critical operation where data loss is unacceptable.

Note

Design evolution: The initial proposal was a heavyweight undo-based TransactionManager/Journal/UndoLog system (~4000 lines). After implementation and review, this was replaced with a lightweight LockManager + RedoLog architecture (~700 lines). The undo/rollback approach was over-engineered for OpenViking's actual consistency needs. See commit 4e44a5d for the refactor rationale.

Architecture

LockContext (async context manager — user-facing entry point)
  └── LockManager (global singleton)
        ├── PathLock (fencing-token-based path locks via .path.ovlock files)
        ├── RedoLog (AGFS-persisted crash recovery for session_memory)
        └── Background cleanup (stale lock detection every 60s)

Key Design Decisions

1. Inline error handling, not rollback: VikingFS rm/mv use sequential operations with explicit error cleanup. Failures are observable and handled at the call site. This is simpler and more predictable than implicit rollback.

2. Selective crash recovery (RedoLog): Only session.commit memory extraction is redo-protected. General FS operations do not need redo because partial failures are detectable and VectorDB inconsistencies self-heal through re-indexing.

3. Fencing tokens for locks: Lock files contain {owner_id}:{monotonic_ns}:{lock_type} for stale lock detection and ABA prevention. Deterministic livelock resolution via timestamp comparison.

4. Fail-fast by default: lock_timeout=0 means operations fail immediately on lock conflict. Configurable for blocking retries.

Lock Design

Lock Type	Purpose	Conflict Detection
POINT (P)	Single directory lock	Checks ancestors for SUBTREE locks
SUBTREE (S)	Recursive directory lock	Scans descendants for any locks

Operation	Lock Mode	Error Handling
rm()	Dir: subtree; File: point(parent)	VectorDB failure → log, still delete FS
mv()	SUBTREE(src) + SUBTREE(dst)	VectorDB failure → delete copy, propagate
add_resource	point(parent dir)	Propagate error
session.commit	Redo-log (no path lock on archive)	Crash → redo marker replayed on restart

Session Commit — Two-Phase with Redo Protection

Phase 1 (Archive): write archive → clear messages
Phase 2 (Memory — redo-protected): write redo marker → extract memories → write → enqueue → mark done

On crash during Phase 2, LockManager.start() replays from the redo marker.

Crash Recovery

On startup, LockManager.start():

1. Scans /local/_system/redo/ for pending markers
2. Replays session memory extraction for each
3. Starts background stale lock cleanup (every 60s)

Scope & Limitations

Important

Single-node only. Distributed scenarios require replacing PathLock with distributed locks (etcd/ZooKeeper) and persisting RedoLog to a consistent distributed store.

Alternatives Considered

• Full undo-based transactions (WAL + UndoLog): Implemented first, then removed. Over-engineered for actual needs — ~4000 lines of code for rollback semantics that were not necessary given OpenViking's eventual consistency model for VectorDB.
• Database-level transactions (SQLite): Only covers VectorDB, not AGFS filesystem operations.
• Saga pattern: More complex orchestration than needed.
• Event sourcing: Overkill for the current write patterns.

Implementation

PR: #431

• 77 files changed, +3732/-1429 lines
• 6 test files covering lock_context, lock_manager, path_lock, redo_log, concurrent_lock, and e2e
• Transaction code reduced from ~4000 lines to ~700 lines (82% reduction)
• Post-refactor fixes: TOCTOU race in lock acquisition, resource lock for add_resource, path handling edge cases

Contribution

• I am willing to contribute to implementing this feature

[r266-tech]

Great design! A few observations from an operational perspective as an OpenViking deployer:

1. Post-action retry policy
The current design defers enqueue_semantic as a post-action replayed on crash recovery. But what about repeated failures (e.g., LLM provider rate-limiting or outage)? A bounded retry with exponential backoff + dead-letter mechanism would prevent infinite replay loops. Something like:

• Max retries: configurable (default 3)
• On exhaustion: move to a _failed_post_actions/ journal for manual inspection

2. Journal compaction / TTL
For long-running instances, /local/_system/transactions/ could accumulate completed journals. A periodic cleanup (e.g., remove COMMITTED journals older than 24h) would keep the directory lean. This matters for start() scan performance.

3. Lock timeout observability
The fencing-token design is solid. One suggestion: emit a structured log/event when a stale lock is detected and force-acquired. This helps operators understand contention patterns without digging into journal files.

4. Real-world consistency issue we've hit
In our deployment, we've observed orphaned VectorDB entries after interrupted rm() operations — exactly the scenario described. The undo-log approach with VectorDB-first ordering makes sense. One edge case worth testing: what happens if the VectorDB snapshot used for rollback is itself stale (e.g., concurrent write to the same URI)?

Looking forward to this landing — it addresses real pain points. Happy to help test.

[r266-tech]

💡 TransactionJournal Contribution Proposal

Hi, I'd like to contribute the TransactionJournal (WAL) component for this feature.

Why TransactionJournal first?

• Highest independence — no dependency on TransactionContext or ResourceManager changes
• Unlocks all downstream components (TransactionContext, ResourceManager integration)
• Complements the likely TransactionContext focus, enabling parallel development

Proposed Design

• JournalEntry dataclass: txn_id, operation (enum: CREATE/UPDATE/DELETE/MOVE), target_uri, old_data/new_data, timestamp, status (PENDING/COMMITTED/ROLLED_BACK)
• JournalSummary: lightweight view for listing/monitoring
• TransactionJournal class with async interface:
  • begin(txn_id) / record(entry) / commit(txn_id) / rollback(txn_id)
  • recover() — replays incomplete transactions on startup
  • compact(before_timestamp) — garbage collection of old committed entries
• Storage: append-only file per transaction under data/transactions/
• Full test coverage with pytest

Scope

Aiming for a focused PR covering:

1. Data models (JournalEntry, JournalSummary, operation enum)
2. TransactionJournal core implementation
3. Recovery mechanism
4. Compaction
5. Unit tests

Happy to discuss design choices or adjust scope. Already have a detailed implementation plan ready.

---

Related: PR #402 (URI normalization) and PR #403 (PDF bookmark headings) are also from me — building familiarity with the codebase.

[qin-ctx]

Hi @r266-tech — thanks for the detailed feedback and the offer to contribute! Great timing to flag this — we're actively working on this feature right now and want to avoid duplicating effort.

Current Progress

The transaction mechanism is largely implemented. Here's what's already in place:

Core implementation (openviking/storage/transaction/):

• TransactionJournal — AGFS-persisted WAL at /local/_system/transactions/, with begin / record_undo / commit / rollback and crash recovery scan on startup (this is done, so the component you proposed is already covered)
• TransactionRecord — data models for journal entries and transaction state machine (INIT → ACQUIRE → EXEC → COMMITTED → RELEASED)
• PathLock — fencing-token-based distributed locks ({tx_id}:{monotonic_ns})
• UndoLog — ordered undo entries with best-effort rollback (each step independently try-caught)
• TransactionContext — async context manager wrapping the full lifecycle
• TransactionManager — orchestrates lifecycle, lock coordination, and crash recovery

Integration: rm(), mv(), add_resource finalize, and two-phase session.commit() are all being wired in.

Tests (tests/transaction/): 52 tests covering unit (undo, journal, path_lock, context_manager) and integration (rm/mv rollback, crash recovery, post_actions, concurrent locks).

On Your Feedback

All four points are well-taken:

1. Post-action retry with backoff + dead-letter — not yet implemented. Retry count/delay fields are reserved in the journal entry, but the bounded-retry + _failed_post_actions/ fallback is missing. Clear gap.
2. Journal compaction / TTL — also not done. The start() scan will get slower over time without it.
3. Stale lock observability — we emit a debug log today, but no structured event. Worth adding before the first production release.
4. Stale VectorDB snapshot on rollback — good edge case. The VectorDB-first ordering in rm() mitigates most scenarios, but a concurrent write to the same URI after the snapshot but before rollback is a real gap we should test explicitly.

Where You Could Contribute

Given the core is built, the most valuable areas for contribution are:

• Post-action retry policy — bounded retries + exponential backoff + _failed_post_actions/ dead-letter directory (your point 1 is already a solid spec)
• Journal compaction — background task or on-startup GC for COMMITTED journals older than a configurable TTL
• Stale-lock structured events — a small observability hook when force-acquiring a lock
• Edge-case tests — the concurrent-write-during-rollback scenario you raised; chaos/fault-injection style tests for crash recovery paths

We'll have a draft PR up soon. Happy to loop you in once it's open so you can pick up one of the above. Does any of these areas interest you?

[r266-tech]

Thanks @qin-ctx for the thorough status update! Great to see the core is already solid.

I'd love to pick up Post-action retry policy — it's the most self-contained piece and I already have a concrete spec in mind:

• Bounded retries with exponential backoff (configurable max_retries, base_delay_s, max_delay_s)
• Dead-letter directory at _failed_post_actions/ for actions that exhaust retries
• Structured logging for each retry attempt + final dead-letter write
• Tests covering: retry exhaustion → dead-letter, transient failure → eventual success, backoff timing

Happy to also tackle journal compaction as a follow-up once the retry policy is merged.

Please loop me in on the draft PR — I'll base my work on it. 🚀

[qin-ctx]

Post-action retry is yours!

Quick context on the current implementation: _execute_post_actions() in transaction_manager.py is single-shot fire-and-forget today — failure logs a warning and the action is dropped. Your spec covers exactly what's missing.

A few design notes before you start:

On retry necessity: crash recovery already handles the "process died before post-actions ran" case — the startup scan replays any COMMITTED journal with pending post-actions. What retry needs to solve is transient failures while the process is still alive (queue temporarily unavailable, enqueue timeout, etc.). So in-memory retry state is sufficient — no need to persist back to the journal.

On retry strategy: recommend only retrying on transient errors (asyncio.TimeoutError, connection-class exceptions) and routing permanent failures (unknown action type, bad params) straight to dead-letter to avoid pointless retries. Suggested backoff: max_retries=3, base_delay=1s, max_delay=30s, with jitter to avoid thundering herd.

Dead-letter path: suggest per-transaction scoping at /local/_system/transactions/{tx_id}/_failed/ — easier to correlate failures back to their originating transaction.

Suggest splitting post-action retry and journal compaction into two separate PRs to keep each reviewable. I'll tag you once the draft PR is up.