From c83d567917267bb3d52708f4b38d2daf36c1f135 Mon Sep 17 00:00:00 2001 From: Paul Hernandez <60959+phernandez@users.noreply.github.com> Date: Fri, 3 Oct 2025 21:10:09 -0500 Subject: [PATCH] fix: enable WAL mode and add Windows-specific SQLite optimizations (#316) Signed-off-by: phernandez Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Paul Hernandez Co-authored-by: Claude --- ...4- Cloud Git Versioning & GitHub Backup.md | 210 ++++++++++ ...-Ups- Conflict, Sync, and Observability.md | 390 ++++++++++++++++++ src/basic_memory/db.py | 107 ++++- test-int/test_db_wal_mode.py | 143 +++++++ 4 files changed, 847 insertions(+), 3 deletions(-) create mode 100644 specs/SPEC-14- Cloud Git Versioning & GitHub Backup.md create mode 100644 specs/SPEC-9-1 Follow-Ups- Conflict, Sync, and Observability.md create mode 100644 test-int/test_db_wal_mode.py diff --git a/specs/SPEC-14- Cloud Git Versioning & GitHub Backup.md b/specs/SPEC-14- Cloud Git Versioning & GitHub Backup.md new file mode 100644 index 00000000..60ceadd5 --- /dev/null +++ b/specs/SPEC-14- Cloud Git Versioning & GitHub Backup.md @@ -0,0 +1,210 @@ +--- +title: 'SPEC-14: Cloud Git Versioning & GitHub Backup' +type: spec +permalink: specs/spec-14-cloud-git-versioning +tags: +- git +- github +- backup +- versioning +- cloud +related: +- specs/spec-9-multi-project-bisync +- specs/spec-9-follow-ups-conflict-sync-and-observability +status: deferred +--- + +# SPEC-14: Cloud Git Versioning & GitHub Backup + +**Status: DEFERRED** - Postponed until multi-user/teams feature development. Using S3 versioning (SPEC-9.1) for v1 instead. + +## Why Deferred + +**Original goals can be met with simpler solutions:** +- Version history → **S3 bucket versioning** (automatic, zero config) +- Offsite backup → **Tigris global replication** (built-in) +- Restore capability → **S3 version restore** (`bm cloud restore --version-id`) +- Collaboration → **Deferred to teams/multi-user feature** (not v1 requirement) + +**Complexity vs value trade-off:** +- Git integration adds: committer service, puller service, webhooks, LFS, merge conflicts +- Risk: Loop detection between Git ↔ rclone bisync ↔ local edits +- S3 versioning gives 80% of value with 5% of complexity + +**When to revisit:** +- Teams/multi-user features (PR-based collaboration workflow) +- User requests for commit messages and branch-based workflows +- Need for fine-grained audit trail beyond S3 object metadata + +--- + +## Original Specification (for reference) + +## Why +Early access users want **transparent version history**, easy **offsite backup**, and a familiar **restore/branching** workflow. Git/GitHub integration would provide: +- Auditable history of every change (who/when/why) +- Branches/PRs for review and collaboration +- Offsite private backup under the user's control +- Escape hatch: users can always `git clone` their knowledge base + +**Note:** These goals are now addressed via S3 versioning (SPEC-9.1) for single-user use case. + +## Goals +- **Transparent**: Users keep using Basic Memory; Git runs behind the scenes. +- **Private**: Push to a **private GitHub repo** that the user owns (or tenant org). +- **Reliable**: No data loss, deterministic mapping of filesystem ↔ Git. +- **Composable**: Plays nicely with SPEC‑9 bisync and upcoming conflict features (SPEC‑9 Follow‑Ups). + +**Non‑Goals (for v1):** +- Fine‑grained per‑file encryption in Git history (can be layered later). +- Large media optimization beyond Git LFS defaults. + +## User Stories +1. *As a user*, I connect my GitHub and choose a private backup repo. +2. *As a user*, every change I make in cloud (or via bisync) is **committed** and **pushed** automatically. +3. *As a user*, I can **restore** a file/folder/project to a prior version. +4. *As a power user*, I can **git pull/push** directly to collaborate outside the app. +5. *As an admin*, I can enforce repo ownership (tenant org) and least‑privilege scopes. + +## Scope +- **In scope:** Full repo backup of `/app/data/` (all projects) with optional selective subpaths. +- **Out of scope (v1):** Partial shallow mirrors; encrypted Git; cross‑provider SCM (GitLab/Bitbucket). + +## Architecture +### Topology +- **Authoritative working tree**: `/app/data/` (bucket mount) remains the source of truth (SPEC‑9). +- **Bare repo** lives alongside: `/app/git/${tenant}/knowledge.git` (server‑side). +- **Mirror remote**: `github.com//.git` (private). + +```mermaid +flowchart LR + A[/Users & Agents/] -->|writes/edits| B[/app/data/] + B -->|file events| C[Committer Service] + C -->|git commit| D[(Bare Repo)] + D -->|push| E[(GitHub Private Repo)] + E -->|webhook (push)| F[Puller Service] + F -->|git pull/merge| D + D -->|checkout/merge| B +``` + +### Services +- **Committer Service** (daemon): + - Watches `/app/data/` for changes (inotify/poll) + - Batches changes (debounce e.g. 2–5s) + - Writes `.bmmeta` (if present) into commit message trailer (see Follow‑Ups) + - `git add -A && git commit -m "chore(sync): + +BM-Meta: "` + - Periodic `git push` to GitHub mirror (configurable interval) +- **Puller Service** (webhook target): + - Receives GitHub webhook (push) → `git fetch` + - **Fast‑forward** merges to `main` only; reject non‑FF unless policy allows + - Applies changes back to `/app/data/` via clean checkout + - Emits sync events for Basic Memory indexers + +### Auth & Security +- **GitHub App** (recommended): minimal scopes: `contents:read/write`, `metadata:read`, webhook. +- Tenant‑scoped installation; repo created in user account or tenant org. +- Tokens stored in KMS/secret manager; rotated automatically. +- Optional policy: allow only **FF merges** on `main`; non‑FF requires PR. + +### Repo Layout +- **Monorepo** (default): one repo per tenant mirrors `/app/data/` with subfolders per project. +- Optional multi‑repo mode (later): one repo per project. + +### File Handling +- Honor `.gitignore` generated from `.bmignore.rclone` + BM defaults (cache, temp, state). +- **Git LFS** for large binaries (images, media) — auto track by extension/size threshold. +- Normalize newline + Unicode (aligns with Follow‑Ups). + +### Conflict Model +- **Primary concurrency**: SPEC‑9 Follow‑Ups (`.bmmeta`, conflict copies) stays the first line of defense. +- **Git merges** are a **secondary** mechanism: + - Server only auto‑merges **text** conflicts when trivial (FF or clean 3‑way). + - Otherwise, create `name (conflict from , ).md` and surface via events. + +### Data Flow vs Bisync +- Bisync (rclone) continues between local sync dir ↔ bucket. +- Git sits **cloud‑side** between bucket and GitHub. +- On **pull** from GitHub → files written to `/app/data/` → picked up by indexers & eventually by bisync back to users. + +## CLI & UX +New commands (cloud mode): +- `bm cloud git connect` — Launch GitHub App installation; create private repo; store installation id. +- `bm cloud git status` — Show connected repo, last push time, last webhook delivery, pending commits. +- `bm cloud git push` — Manual push (rarely needed). +- `bm cloud git pull` — Manual pull/FF (admin only by default). +- `bm cloud snapshot -m "message"` — Create a tagged point‑in‑time snapshot (git tag). +- `bm restore --to ` — Restore file/folder/project to prior version. + +Settings: +- `bm config set git.autoPushInterval=5s` +- `bm config set git.lfs.sizeThreshold=10MB` +- `bm config set git.allowNonFF=false` + +## Migration & Backfill +- On connect, if repo empty: initial commit of entire `/app/data/`. +- If repo has content: require **one‑time import** path (clone to staging, reconcile, choose direction). + +## Edge Cases +- Massive deletes: gated by SPEC‑9 `max_delete` **and** Git pre‑push hook checks. +- Case changes and rename detection: rely on git rename heuristics + Follow‑Ups move hints. +- Secrets: default ignore common secret patterns; allow custom deny list. + +## Telemetry & Observability +- Emit `git_commit`, `git_push`, `git_pull`, `git_conflict` events with correlation IDs. +- `bm sync --report` extended with Git stats (commit count, delta bytes, push latency). + +## Phased Plan +### Phase 0 — Prototype (1 sprint) +- Server: bare repo init + simple committer (batch every 10s) + manual GitHub token. +- CLI: `bm cloud git connect --token ` (dev‑only) +- Success: edits in `/app/data/` appear in GitHub within 30s. + +### Phase 1 — GitHub App & Webhooks (1–2 sprints) +- Switch to GitHub App installs; create private repo; store installation id. +- Committer hardened (debounce 2–5s, backoff, retries). +- Puller service with webhook → FF merge → checkout to `/app/data/`. +- LFS auto‑track + `.gitignore` generation. +- CLI surfaces status + logs. + +### Phase 2 — Restore & Snapshots (1 sprint) +- `bm restore` for file/folder/project with dry‑run. +- `bm cloud snapshot` tags + list/inspect. +- Policy: PR‑only non‑FF, admin override. + +### Phase 3 — Selective & Multi‑Repo (nice‑to‑have) +- Include/exclude projects; optional per‑project repos. +- Advanced policies (branch protections, required reviews). + +## Acceptance Criteria +- Changes to `/app/data/` are committed and pushed automatically within configurable interval (default ≤5s). +- GitHub webhook pull results in updated files in `/app/data/` (FF‑only by default). +- LFS configured and functioning; large files don't bloat history. +- `bm cloud git status` shows connected repo and last push/pull times. +- `bm restore` restores a file/folder to a prior commit with a clear audit trail. +- End‑to‑end works alongside SPEC‑9 bisync without loops or data loss. + +## Risks & Mitigations +- **Loop risk (Git ↔ Bisync)**: Writes to `/app/data/` → bisync → local → user edits → back again. *Mitigation*: Debounce, commit squashing, idempotent `.bmmeta` versioning, and watch exclusion windows during pull. +- **Repo bloat**: Lots of binary churn. *Mitigation*: default LFS, size threshold, optional media‑only repo later. +- **Security**: Token leakage. *Mitigation*: GitHub App with short‑lived tokens, KMS storage, scoped permissions. +- **Merge complexity**: Non‑trivial conflicts. *Mitigation*: prefer FF; otherwise conflict copies + events; require PR for non‑FF. + +## Open Questions +- Do we default to **monorepo** per tenant, or offer project‑per‑repo at connect time? +- Should `restore` write to a branch and open a PR, or directly modify `main`? +- How do we expose Git history in UI (timeline view) without users dropping to CLI? + +## Appendix: Sample Config +```json +{ + "git": { + "enabled": true, + "repo": "https://github.com//.git", + "autoPushInterval": "5s", + "allowNonFF": false, + "lfs": { "sizeThreshold": 10485760 } + } +} +``` diff --git a/specs/SPEC-9-1 Follow-Ups- Conflict, Sync, and Observability.md b/specs/SPEC-9-1 Follow-Ups- Conflict, Sync, and Observability.md new file mode 100644 index 00000000..ee8314a8 --- /dev/null +++ b/specs/SPEC-9-1 Follow-Ups- Conflict, Sync, and Observability.md @@ -0,0 +1,390 @@ +--- +title: 'SPEC-9-1 Follow-Ups: Conflict, Sync, and Observability' +type: tasklist +permalink: specs/spec-9-follow-ups-conflict-sync-and-observability +related: specs/spec-9-multi-project-bisync +status: revised +revision_date: 2025-10-03 +--- + +# SPEC-9-1 Follow-Ups: Conflict, Sync, and Observability + +**REVISED 2025-10-03:** Simplified to leverage rclone built-ins instead of custom conflict handling. + +**Context:** SPEC-9 delivered multi-project bidirectional sync and a unified CLI. This follow-up focuses on **observability and safety** using rclone's built-in capabilities rather than reinventing conflict handling. + +**Design Philosophy: "Be Dumb Like Git"** +- Let rclone bisync handle conflict detection (it already does this) +- Make conflicts visible and recoverable, don't prevent them +- Cloud is always the winner on conflict (cloud-primary model) +- Users who want version history can just use Git locally in their sync directory + +**What Changed from Original Version:** +- **Replaced:** Custom `.bmmeta` sidecars → Use rclone's `.bisync/` state tracking +- **Replaced:** Custom conflict detection → Use rclone bisync 3-way merge +- **Replaced:** Tombstone files → rclone delete tracking handles this +- **Replaced:** Distributed lease → Local process lock only (document multi-device warning) +- **Replaced:** S3 versioning service → Users just use Git locally if they want history +- **Deferred:** SPEC-14 Git integration → Postponed to teams/multi-user features + +## ✅ Now +- [ ] **Local process lock**: Prevent concurrent bisync runs on same device (`~/.basic-memory/sync.lock`) +- [ ] **Structured sync reports**: Parse rclone bisync output into JSON reports (creates/updates/deletes/conflicts, bytes, duration); `bm sync --report` +- [ ] **Multi-device warning**: Document that users should not run `--watch` on multiple devices simultaneously +- [ ] **Version control guidance**: Document pattern for users to use Git locally in their sync directory if they want version history +- [ ] **Docs polish**: cloud-mode toggle, mount↔bisync directory isolation, conflict semantics, quick start, migration guide, short demo clip/GIF + +## 🔜 Next +- [ ] **Observability commands**: `bm conflicts list`, `bm sync history` to view sync reports and conflicts +- [ ] **Conflict resolution UI**: `bm conflicts resolve ` to interactively pick winner from conflict files +- [ ] **Selective sync**: allow include/exclude by project; per-project profile (safe/balanced/fast) + +## 🧭 Later +- [ ] **Near real-time sync**: File watcher → targeted `rclone copy` for individual files (keep bisync as backstop) +- [ ] **Sharing / scoped tokens**: cross-tenant/project access +- [ ] **Bandwidth controls & backpressure**: policy for large repos +- [ ] **Client-side encryption (optional)**: with clear trade-offs + +## 📏 Acceptance criteria (for "Now" items) +- [ ] Local process lock prevents concurrent bisync runs on same device +- [ ] rclone bisync conflict files visible and documented (`file.conflict1.md`, `file.conflict2.md`) +- [ ] `bm sync --report` generates parsable JSON with sync statistics +- [ ] Documentation clearly warns about multi-device `--watch` mode +- [ ] Documentation shows users how to use Git locally for version history + +## What We're NOT Building (Deferred to rclone) +- ❌ Custom `.bmmeta` sidecars (rclone tracks state in `.bisync/` workdir) +- ❌ Custom conflict detection (rclone bisync already does 3-way merge detection) +- ❌ Tombstone files (S3 versioning + rclone delete tracking handles this) +- ❌ Distributed lease (low probability issue, rclone detects state divergence) +- ❌ Rename/move tracking (rclone has size+modtime heuristics built-in) + +## Implementation Summary + +**Current State (SPEC-9):** +- ✅ rclone bisync with 3 profiles (safe/balanced/fast) +- ✅ `--max-delete` safety limits (10/25/50 files) +- ✅ `--conflict-resolve=newer` for auto-resolution +- ✅ Watch mode: `bm sync --watch` (60s intervals) +- ✅ Integrity checking: `bm cloud check` +- ✅ Mount vs bisync directory isolation + +**What's Needed (This Spec):** +1. **Process lock** - Simple file-based lock in `~/.basic-memory/sync.lock` +2. **Sync reports** - Parse rclone output, save to `~/.basic-memory/sync-history/` +3. **Documentation** - Multi-device warnings, conflict resolution workflow, Git usage pattern + +**User Model:** +- Cloud is always the winner on conflict (cloud-primary) +- rclone creates `.conflict` files for divergent edits +- Users who want version history just use Git in their local sync directory +- Users warned: don't run `--watch` on multiple devices + +## Decision Rationale & Trade-offs + +### Why Trust rclone Instead of Custom Conflict Handling? + +**rclone bisync already provides:** +- 3-way merge detection (compares local, remote, and last-known state) +- File state tracking in `.bisync/` workdir (hashes, modtimes) +- Automatic conflict file creation: `file.conflict1.md`, `file.conflict2.md` +- Rename detection via size+modtime heuristics +- Delete tracking (prevents resurrection of deleted files) +- Battle-tested with extensive edge case handling + +**What we'd have to build with custom approach:** +- Per-file metadata tracking (`.bmmeta` sidecars) +- 3-way diff algorithm +- Conflict detection logic +- Tombstone files for deletes +- Rename/move detection +- Testing for all edge cases + +**Decision:** Use what rclone already does well. Don't reinvent the wheel. + +### Why Let Users Use Git Locally Instead of Building Versioning? + +**The simplest solution: Just use Git** + +Users who want version history can literally just use Git in their sync directory: + +```bash +cd ~/basic-memory-cloud-sync/ +git init +git add . +git commit -m "backup" + +# Push to their own GitHub if they want +git remote add origin git@github.com:user/my-knowledge.git +git push +``` + +**Why this is perfect:** +- ✅ We build nothing +- ✅ Users who want Git... just use Git +- ✅ Users who don't care... don't need to +- ✅ rclone bisync already handles sync conflicts +- ✅ Users own their data, they can version it however they want (Git, Time Machine, etc.) + +**What we'd have to build for S3 versioning:** +- API to enable versioning on Tigris buckets + - **Problem**: Tigris doesn't support S3 bucket versioning +- Restore commands: `bm cloud restore --version-id` +- Version listing: `bm cloud versions ` +- Lifecycle policies for version retention +- Documentation and user education + +**What we'd have to build for SPEC-14 Git integration:** +- Committer service (daemon watching `/app/data/`) +- Puller service (webhook handler for GitHub pushes) +- Git LFS for large files +- Loop prevention between Git ↔ bisync ↔ local +- Merge conflict handling at TWO layers (rclone + Git) +- Webhook infrastructure and monitoring + +**Decision:** Don't build version control. Document the pattern. "The easiest problem to solve is the one you avoid." + +**When to revisit:** Teams/multi-user features where server-side version control becomes necessary for collaboration. + +### Why No Distributed Lease? + +**Low probability issue:** +- Requires user to manually run `bm sync` on multiple devices at exact same time +- Most users run `--watch` on one primary device +- rclone bisync detects state divergence and fails safely + +**Safety nets in place:** +- Local process lock prevents concurrent runs on same device +- rclone bisync aborts if bucket state changed during sync +- S3 versioning recovers from any overwrites +- Documentation warns against multi-device `--watch` + +**Failure mode:** +```bash +# Device A and B sync simultaneously +Device A: bm sync → succeeds +Device B: bm sync → "Error: path has changed, run --resync" + +# User fixes with resync +Device B: bm sync --resync → establishes new baseline +``` + +**Decision:** Document the issue, add local lock, defer distributed coordination until users report actual problems. + +### Cloud-Primary Conflict Model + +**User mental model:** +- Cloud is the source of truth (like Dropbox/iCloud) +- Local is working copy +- On conflict: cloud wins, local edits → `.conflict` file +- User manually picks winner + +**Why this works:** +- Simpler than bidirectional merge (no automatic resolution risk) +- Matches user expectations from Dropbox +- S3 versioning provides safety net for overwrites +- Clear recovery path: restore from S3 version if needed + +**Example workflow:** +```bash +# Edit file on Device A and Device B while offline +# Both devices come online and sync + +Device A: bm sync +# → Pushes to cloud first, becomes canonical version + +Device B: bm sync +# → Detects conflict +# → Cloud version: work/notes.md +# → Local version: work/notes.md.conflict1 +# → User manually merges or picks winner + +# Restore if needed +bm cloud restore work/notes.md --version-id abc123 +``` + +## Implementation Details + +### 1. Local Process Lock + +```python +# ~/.basic-memory/sync.lock +import os +import psutil +from pathlib import Path + +class SyncLock: + def __init__(self): + self.lock_file = Path.home() / '.basic-memory' / 'sync.lock' + + def acquire(self): + if self.lock_file.exists(): + pid = int(self.lock_file.read_text()) + if psutil.pid_exists(pid): + raise BisyncError( + f"Sync already running (PID {pid}). " + f"Wait for completion or kill stale process." + ) + # Stale lock, remove it + self.lock_file.unlink() + + self.lock_file.write_text(str(os.getpid())) + + def release(self): + if self.lock_file.exists(): + self.lock_file.unlink() + + def __enter__(self): + self.acquire() + return self + + def __exit__(self, *args): + self.release() + +# Usage +with SyncLock(): + run_rclone_bisync() +``` + +### 3. Sync Report Parsing + +```python +# Parse rclone bisync output +import json +from datetime import datetime +from pathlib import Path + +def parse_sync_report(rclone_output: str, duration: float, exit_code: int) -> dict: + """Parse rclone bisync output into structured report.""" + + # rclone bisync outputs lines like: + # "Synching Path1 /local/path with Path2 remote:bucket" + # "- Path1 File was copied to Path2" + # "Bisync successful" + + report = { + "timestamp": datetime.now().isoformat(), + "duration_seconds": duration, + "exit_code": exit_code, + "success": exit_code == 0, + "files_created": 0, + "files_updated": 0, + "files_deleted": 0, + "conflicts": [], + "errors": [] + } + + for line in rclone_output.split('\n'): + if 'was copied to' in line: + report['files_created'] += 1 + elif 'was updated in' in line: + report['files_updated'] += 1 + elif 'was deleted from' in line: + report['files_deleted'] += 1 + elif '.conflict' in line: + report['conflicts'].append(line.strip()) + elif 'ERROR' in line: + report['errors'].append(line.strip()) + + return report + +def save_sync_report(report: dict): + """Save sync report to history.""" + history_dir = Path.home() / '.basic-memory' / 'sync-history' + history_dir.mkdir(parents=True, exist_ok=True) + + timestamp = datetime.now().strftime('%Y%m%d-%H%M%S') + report_file = history_dir / f'{timestamp}.json' + + report_file.write_text(json.dumps(report, indent=2)) + +# Usage in run_bisync() +start_time = time.time() +result = subprocess.run(bisync_cmd, capture_output=True, text=True) +duration = time.time() - start_time + +report = parse_sync_report(result.stdout, duration, result.returncode) +save_sync_report(report) + +if report['conflicts']: + console.print(f"[yellow]⚠ {len(report['conflicts'])} conflict(s) detected[/yellow]") + console.print("[dim]Run 'bm conflicts list' to view[/dim]") +``` + +### 4. User Commands + +```bash +# View sync history +bm sync history +# → Lists recent syncs from ~/.basic-memory/sync-history/*.json +# → Shows: timestamp, duration, files changed, conflicts, errors + +# View current conflicts +bm conflicts list +# → Scans sync directory for *.conflict* files +# → Shows: file path, conflict versions, timestamps + +# Restore from S3 version +bm cloud restore work/notes.md --version-id abc123 +# → Uses aws s3api get-object with version-id +# → Downloads to original path + +bm cloud restore work/notes.md --timestamp "2025-10-03 14:30" +# → Lists versions, finds closest to timestamp +# → Downloads that version + +# List file versions +bm cloud versions work/notes.md +# → Uses aws s3api list-object-versions +# → Shows: version-id, timestamp, size, author + +# Interactive conflict resolution +bm conflicts resolve work/notes.md +# → Shows both versions side-by-side +# → Prompts: Keep local, keep cloud, merge manually, restore from S3 version +# → Cleans up .conflict files after resolution +``` + +## Success Metrics & Monitoring + +**Phase 1 (v1) - Basic Safety:** +- [ ] Conflict detection rate < 5% of syncs (measure in telemetry) +- [ ] User can resolve conflicts within 5 minutes (UX testing) +- [ ] Documentation prevents 90% of multi-device issues + +**Phase 2 (v2) - Observability:** +- [ ] 80% of users check `bm sync history` when troubleshooting +- [ ] Average time to restore from S3 version < 2 minutes +- +- [ ] Conflict resolution success rate > 95% + +**What to measure:** +```python +# Telemetry in sync reports +{ + "conflict_rate": conflicts / total_syncs, + "multi_device_collisions": count_state_divergence_errors, + "version_restores": count_restore_operations, + "avg_sync_duration": sum(durations) / count, + "max_delete_trips": count_max_delete_aborts +} +``` + +**When to add distributed lease:** +- Multi-device collision rate > 5% of syncs +- User complaints about state divergence errors +- Evidence that local lock isn't sufficient + +**When to revisit Git (SPEC-14):** +- Teams feature launches (multi-user collaboration) +- Users request commit messages / audit trail +- PR-based review workflow becomes valuable + +## Links +- SPEC-9: `specs/spec-9-multi-project-bisync` +- SPEC-14: `specs/spec-14-cloud-git-versioning` (deferred in favor of S3 versioning) +- rclone bisync docs: https://rclone.org/bisync/ +- Tigris S3 versioning: https://www.tigrisdata.com/docs/buckets/versioning/ + +--- +**Owner:** | **Review cadence:** weekly in standup | **Last updated:** 2025-10-03 diff --git a/src/basic_memory/db.py b/src/basic_memory/db.py index 9cff240a..f8d19f2c 100644 --- a/src/basic_memory/db.py +++ b/src/basic_memory/db.py @@ -1,4 +1,5 @@ import asyncio +import os from contextlib import asynccontextmanager from enum import Enum, auto from pathlib import Path @@ -9,7 +10,7 @@ from alembic import command from alembic.config import Config from loguru import logger -from sqlalchemy import text +from sqlalchemy import text, event from sqlalchemy.ext.asyncio import ( create_async_engine, async_sessionmaker, @@ -17,6 +18,7 @@ from sqlalchemy.ext.asyncio import ( AsyncEngine, async_scoped_session, ) +from sqlalchemy.pool import NullPool from basic_memory.repository.search_repository import SearchRepository @@ -73,13 +75,77 @@ async def scoped_session( await factory.remove() +def _configure_sqlite_connection(dbapi_conn, enable_wal: bool = True) -> None: + """Configure SQLite connection with WAL mode and optimizations. + + Args: + dbapi_conn: Database API connection object + enable_wal: Whether to enable WAL mode (should be False for in-memory databases) + """ + cursor = dbapi_conn.cursor() + try: + # Enable WAL mode for better concurrency (not supported for in-memory databases) + if enable_wal: + cursor.execute("PRAGMA journal_mode=WAL") + # Set busy timeout to handle locked databases + cursor.execute("PRAGMA busy_timeout=10000") # 10 seconds + # Optimize for performance + cursor.execute("PRAGMA synchronous=NORMAL") + cursor.execute("PRAGMA cache_size=-64000") # 64MB cache + cursor.execute("PRAGMA temp_store=MEMORY") + # Windows-specific optimizations + if os.name == "nt": + cursor.execute("PRAGMA locking_mode=NORMAL") # Ensure normal locking on Windows + except Exception as e: + # Log but don't fail - some PRAGMAs may not be supported + logger.warning(f"Failed to configure SQLite connection: {e}") + finally: + cursor.close() + + def _create_engine_and_session( db_path: Path, db_type: DatabaseType = DatabaseType.FILESYSTEM ) -> tuple[AsyncEngine, async_sessionmaker[AsyncSession]]: """Internal helper to create engine and session maker.""" db_url = DatabaseType.get_db_url(db_path, db_type) logger.debug(f"Creating engine for db_url: {db_url}") - engine = create_async_engine(db_url, connect_args={"check_same_thread": False}) + + # Configure connection args with Windows-specific settings + connect_args: dict[str, bool | float | None] = {"check_same_thread": False} + + # Add Windows-specific parameters to improve reliability + if os.name == "nt": # Windows + connect_args.update( + { + "timeout": 30.0, # Increase timeout to 30 seconds for Windows + "isolation_level": None, # Use autocommit mode + } + ) + # Use NullPool for Windows filesystem databases to avoid connection pooling issues + # Important: Do NOT use NullPool for in-memory databases as it will destroy the database + # between connections + if db_type == DatabaseType.FILESYSTEM: + engine = create_async_engine( + db_url, + connect_args=connect_args, + poolclass=NullPool, # Disable connection pooling on Windows + echo=False, + ) + else: + # In-memory databases need connection pooling to maintain state + engine = create_async_engine(db_url, connect_args=connect_args) + else: + engine = create_async_engine(db_url, connect_args=connect_args) + + # Enable WAL mode for better concurrency and reliability + # Note: WAL mode is not supported for in-memory databases + enable_wal = db_type != DatabaseType.MEMORY + + @event.listens_for(engine.sync_engine, "connect") + def enable_wal_mode(dbapi_conn, connection_record): + """Enable WAL mode on each connection.""" + _configure_sqlite_connection(dbapi_conn, enable_wal=enable_wal) + session_maker = async_sessionmaker(engine, expire_on_commit=False) return engine, session_maker @@ -140,7 +206,42 @@ async def engine_session_factory( db_url = DatabaseType.get_db_url(db_path, db_type) logger.debug(f"Creating engine for db_url: {db_url}") - _engine = create_async_engine(db_url, connect_args={"check_same_thread": False}) + # Configure connection args with Windows-specific settings + connect_args: dict[str, bool | float | None] = {"check_same_thread": False} + + # Add Windows-specific parameters to improve reliability + if os.name == "nt": # Windows + connect_args.update( + { + "timeout": 30.0, # Increase timeout to 30 seconds for Windows + "isolation_level": None, # Use autocommit mode + } + ) + # Use NullPool for Windows filesystem databases to avoid connection pooling issues + # Important: Do NOT use NullPool for in-memory databases as it will destroy the database + # between connections + if db_type == DatabaseType.FILESYSTEM: + _engine = create_async_engine( + db_url, + connect_args=connect_args, + poolclass=NullPool, # Disable connection pooling on Windows + echo=False, + ) + else: + # In-memory databases need connection pooling to maintain state + _engine = create_async_engine(db_url, connect_args=connect_args) + else: + _engine = create_async_engine(db_url, connect_args=connect_args) + + # Enable WAL mode for better concurrency and reliability + # Note: WAL mode is not supported for in-memory databases + enable_wal = db_type != DatabaseType.MEMORY + + @event.listens_for(_engine.sync_engine, "connect") + def enable_wal_mode(dbapi_conn, connection_record): + """Enable WAL mode on each connection.""" + _configure_sqlite_connection(dbapi_conn, enable_wal=enable_wal) + try: _session_maker = async_sessionmaker(_engine, expire_on_commit=False) diff --git a/test-int/test_db_wal_mode.py b/test-int/test_db_wal_mode.py new file mode 100644 index 00000000..3554af4d --- /dev/null +++ b/test-int/test_db_wal_mode.py @@ -0,0 +1,143 @@ +"""Integration tests for WAL mode and Windows-specific SQLite optimizations. + +These tests use real filesystem databases (not in-memory) to verify WAL mode +and other SQLite configuration settings work correctly in production scenarios. +""" + +import pytest +from unittest.mock import patch +from sqlalchemy import text + + +@pytest.mark.asyncio +async def test_wal_mode_enabled(engine_factory): + """Test that WAL mode is enabled on filesystem database connections.""" + engine, _ = engine_factory + + # Execute a query to verify WAL mode is enabled + async with engine.connect() as conn: + result = await conn.execute(text("PRAGMA journal_mode")) + journal_mode = result.fetchone()[0] + + # WAL mode should be enabled for filesystem databases + assert journal_mode.upper() == "WAL" + + +@pytest.mark.asyncio +async def test_busy_timeout_configured(engine_factory): + """Test that busy timeout is configured for database connections.""" + engine, _ = engine_factory + + async with engine.connect() as conn: + result = await conn.execute(text("PRAGMA busy_timeout")) + busy_timeout = result.fetchone()[0] + + # Busy timeout should be 10 seconds (10000 milliseconds) + assert busy_timeout == 10000 + + +@pytest.mark.asyncio +async def test_synchronous_mode_configured(engine_factory): + """Test that synchronous mode is set to NORMAL for performance.""" + engine, _ = engine_factory + + async with engine.connect() as conn: + result = await conn.execute(text("PRAGMA synchronous")) + synchronous = result.fetchone()[0] + + # Synchronous should be NORMAL (1) + assert synchronous == 1 + + +@pytest.mark.asyncio +async def test_cache_size_configured(engine_factory): + """Test that cache size is configured for performance.""" + engine, _ = engine_factory + + async with engine.connect() as conn: + result = await conn.execute(text("PRAGMA cache_size")) + cache_size = result.fetchone()[0] + + # Cache size should be -64000 (64MB) + assert cache_size == -64000 + + +@pytest.mark.asyncio +async def test_temp_store_configured(engine_factory): + """Test that temp_store is set to MEMORY.""" + engine, _ = engine_factory + + async with engine.connect() as conn: + result = await conn.execute(text("PRAGMA temp_store")) + temp_store = result.fetchone()[0] + + # temp_store should be MEMORY (2) + assert temp_store == 2 + + +@pytest.mark.asyncio +async def test_windows_locking_mode_when_on_windows(tmp_path): + """Test that Windows-specific locking mode is set when running on Windows.""" + from basic_memory.db import engine_session_factory, DatabaseType + + db_path = tmp_path / "test_windows.db" + + with patch("os.name", "nt"): + # Need to patch at module level where it's imported + with patch("basic_memory.db.os.name", "nt"): + async with engine_session_factory(db_path, DatabaseType.FILESYSTEM) as ( + engine, + _, + ): + async with engine.connect() as conn: + result = await conn.execute(text("PRAGMA locking_mode")) + locking_mode = result.fetchone()[0] + + # Locking mode should be NORMAL on Windows + assert locking_mode.upper() == "NORMAL" + + +@pytest.mark.asyncio +async def test_null_pool_on_windows(tmp_path): + """Test that NullPool is used on Windows to avoid connection pooling issues.""" + from basic_memory.db import engine_session_factory, DatabaseType + from sqlalchemy.pool import NullPool + + db_path = tmp_path / "test_windows_pool.db" + + with patch("basic_memory.db.os.name", "nt"): + async with engine_session_factory(db_path, DatabaseType.FILESYSTEM) as (engine, _): + # Engine should be using NullPool on Windows + assert isinstance(engine.pool, NullPool) + + +@pytest.mark.asyncio +async def test_regular_pool_on_non_windows(tmp_path): + """Test that regular pooling is used on non-Windows platforms.""" + from basic_memory.db import engine_session_factory, DatabaseType + from sqlalchemy.pool import NullPool + + db_path = tmp_path / "test_posix_pool.db" + + with patch("basic_memory.db.os.name", "posix"): + async with engine_session_factory(db_path, DatabaseType.FILESYSTEM) as (engine, _): + # Engine should NOT be using NullPool on non-Windows + assert not isinstance(engine.pool, NullPool) + + +@pytest.mark.asyncio +async def test_memory_database_no_null_pool_on_windows(tmp_path): + """Test that in-memory databases do NOT use NullPool even on Windows. + + NullPool closes connections immediately, which destroys in-memory databases. + This test ensures in-memory databases maintain connection pooling. + """ + from basic_memory.db import engine_session_factory, DatabaseType + from sqlalchemy.pool import NullPool + + db_path = tmp_path / "test_memory.db" + + with patch("basic_memory.db.os.name", "nt"): + async with engine_session_factory(db_path, DatabaseType.MEMORY) as (engine, _): + # In-memory databases should NOT use NullPool on Windows + assert not isinstance(engine.pool, NullPool)