Bookhoard/bookhoard
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 12 Wiki Activity
904 Commits 5 Branches 13 Tags
b3263b2611ca7aa0345e32a93e5142dbd2f8aec2
Commit Graph
8 Commits
Author SHA1 Message Date
john-okeefe 605aff104b docs: Fix Phase 1 job type duplication with Phase 0.5 
Fixed issue where Phase 1 tried to add job types that were already added in Phase 0.5.

Changes:
1. Step 1.1 - Updated title from 'Add All Job Type Constants' to 'Add NEW Job Type Constants'
   - Now shows current state after Phase 0.5 (JobTypeScan, JobTypeSetFolders, JobTypeDirectoryScan)
   - Only adds NEW Phase 1 job types: Import, Convert, Thumbnails, Reindex, Backup, Analytics, Sync
   - Clarifies that JobTypeScan, JobTypeSetFolders, JobTypeDirectoryScan were added in Phase 0.5

2. Step 1.2 - Updated title from 'Add Job Handlers to Switch Statement' to 'Add NEW Job Handlers to Switch Statement'
   - Now shows current state after Phase 0.5 (handlers for Scan, SetFolders, DirectoryScan)
   - Only adds NEW Phase 1 handlers for the new job types
   - Clarifies that existing handlers were added in Phase 0.5

Impact: Developers now have clear guidance on which job types/handlers to add in each phase, avoiding confusion and potential merge conflicts.
 2026-03-05 13:57:48 -05:00
john-okeefe f2e5114813 docs: Fix critical inconsistencies in Phase 0.5 plan 
Fixed issues identified during review:

1. Commit message accuracy (lines 1207, 1209):
   - Changed 'worker *Worker (job queue reference)' to 'WorkerInstance *Worker global (no circular dependency)'
   - Changed 'fileStability map[string]atomic.Bool' to 'fileStability map[string]*atomic.Bool (pointer)'
   - Removed claim that worker field was ADDED (it was REMOVED in clean rewrite)

2. Polling interval consistency (60s chosen):
   - Constructor: 60s (correct, no change)
   - Test: Changed from expecting 300s to 60s
   - Commit message: Changed all references from 300s to 60s
   - Benefits: 'Delete detection via 60s polling (fast safety net)'
   - Rationale: Real-time fsnotify + 60s polling = best UX

3. Added Step 0.5.3.8: Initialize WorkerInstance in main():
   - Previously buried as inline comment in Step 0.5.3.7
   - Now dedicated step with file location (cmd/server/main.go)
   - Critical for system initialization

4. Removed duplicate benefits lines:
   - Lines 1252-1254 were duplicates of 1249-1251

5. Updated 'Code to ADD' section:
   - Clarified '*atomic.Bool (pointer to atomic.Bool, not value type)'
   - Clarified 'WorkerInstance *Worker global (no circular dependency)'
   - Added 'JobTypeDirectoryScan' to constants list

6. Updated Files modified section:
   - Added cmd/server/main.go (initialize WorkerInstance)
   - Clarified worker.go changes (JobTypeDirectoryScan, processDirectoryScanJob, WorkerInstance, Enqueue)
   - Changed scan_settings_integration_test.go description to 'test expects 60s polling'

7. Enhanced Concurrency Control section:
   - Added 'No circular dependency (WorkerInstance global)'

Result: Plan now accurately reflects clean architecture approach with 60s polling.
 2026-03-05 13:04:05 -05:00
john-okeefe 1a552e03f1 docs: Rewrite Phase 0.5 with clean architecture (Phase 0.5 + Phase 1 robustness) 
Critical rewrite to fix broken hybrid approach that tried to merge two incompatible systems.

PROBLEM WITH PREVIOUS APPROACH:
- Tried to use job queue AND direct scanning simultaneously
- Created job parameters that didn't match handler expectations
- Referenced non-existent activeScans map
- Never-initialized worker field in MediaScanner
- performInitialScan() bypassed job queue
- Like building a car with parts from two different manufacturers

CLEAN ARCHITECTURE:
- Job queue handles concurrency control ONLY
- Scanner handles all scanning logic
- Global WorkerInstance provides access (no circular dependency)
- Simple scan_mutex for double-protection
- Clear separation of concerns

KEY CHANGES:
1. MediaScanner struct:
   - Removed: worker *Worker field (circular dependency)
   - Removed: activeScans map (too complex)
   - Fixed: fileStability map[string]*atomic.Bool (was value, now pointer)
   - Added: scan_mutex sync.Mutex (simple, effective)

2. Job queue integration:
   - processDirtyDirectories() submits jobs to WorkerInstance
   - Job parameters: {directory: dirPath, db: s.db}
   - Added JobTypeDirectoryScan constant
   - Added processDirectoryScanJob() handler in Worker
   - performInitialScan() submits jobs (not direct calls)

3. Worker changes:
   - Added WorkerInstance *Worker global variable
   - Added Enqueue() method (non-blocking with fallback)
   - processDirectoryScanJob() creates scanner, calls scanDirectory()

PRESERVED FROM PHASE 0.5:
- Directory watching with dirty dirs tracking
- File stability checks (Audiobookshelf approach)
- Smart event merging (Jellyfin approach)
- 10-second batch processing
- 60-second polling fallback

ADDED FROM PHASE 1 ROBUSTNESS:
- Job queue for concurrency control
- Test isolation
- Fixed default values (30s → 60s)

RESULT:
- No parameter mismatches
- No non-existent fields
- No memory leaks
- Clean separation of concerns
- Best of both worlds without the complexity
 2026-03-05 12:55:48 -05:00
john-okeefe eecfb52996 docs: Incorporate old Phase 1 features into Phase 0.5 
Incorporates all old Phase 1 job queue concurrency control features
into Phase 0.5 to fix fsnotify reliability issues comprehensively.

Features Added from Old Phase 1:
- Job queue for all directory scans (serialized by worker pool)
- JobTypeSetFolders: Async folder configuration via job queue
- processSetFoldersJob() handler
- Test isolation: Snapshot/restore system_settings
- Fix GetPollInterval() default value: 30s → 60s (matches handler/schema)
- Fix test expectation: 60 → 300 (5 min polling interval)

Critical Fixes:
- File stability race condition: Uses atomic.Bool to prevent concurrent checks
- Double-unlock bug: Removed defer unlock, use explicit cleanup only
- Unbounded goroutine spawn: Job queue serializes scans (no semaphore needed)
- activeScans inconsistency: Removed (job queue handles concurrency)
- Worker constructor conflicts: Use Phase 4's signature (db parameter)

Concurrency Control (Job Queue Approach):
- All directory scans submitted as jobs to worker pool
- Worker pool serializes scans naturally (no concurrent access)
- No unbounded goroutine spawn (worker limits concurrency)
- Atomic file stability checks prevent duplicate entries
- No race conditions in fileStability map
- No memory leaks from orphaned map entries

Phase 0.5 Time Estimate: 3-4 hours (was 2-3 hours)
- Added JobTypeSetFolders integration
- Added test isolation implementation
- Fixed all critical issues (double-unlock, race conditions, etc.)

This plan now incorporates the best of both approaches:
- Directory-based watching (Jellyfin)
- File stability checks (Audiobookshelf)
- Job queue concurrency control (old Phase 1)
 2026-03-05 12:38:31 -05:00
john-okeefe 7ff565a068 docs: Fix 12 critical issues in Phase 0.5 fsnotify implementation 
CRITICAL FIXES (would cause test failures):
1. Integration test timing: 5s → 12s
   - Test waited 5s but implementation uses 10s batch delay
   - Would fail intermittently detecting all 20 files

2. Race condition in fileStability map access
   - Lock released between check and insert (lines 342-352)
   - Concurrent calls could create duplicate map entries
   - Fixed by holding lock during entire function

3. Missing concurrency protection in scanDirectory()
   - Multiple scans of same directory could run simultaneously
   - Could cause race conditions in fileStability map
   - Fixed with activeScans map to prevent duplicate scans

MAJOR FIXES (production issues under load):
4. Unbounded goroutine spawn
   - Spawns unlimited goroutines for directory scans
   - 100 changed directories = 100 concurrent scans = 1000s of goroutines
   - Fixed with semaphore limiting concurrent scans to 10
   - You correctly identified this as the same problem Phase 1 job queue solved

5. Memory leak in fileStability map
   - Entries never cleaned up if waitForFileStability() called concurrently
   - Fixed by proper lock pattern and cleanup on all code paths

6. No initial scan of root folders
   - Only watches for NEW changes, misses existing files
   - Fixed by adding performInitialScan() function

MEDIUM FIXES (edge cases / code quality):
7. Removed unused batchTimeout variable
   - Was declared but never actually used

8. Completed smart event merging
   - Added sibling directory consolidation logic
   - Prevents redundant scans of sibling folders

9. Clarified subdirectory handling
   - Updated comment to explain subdirs trigger own events
   - scanDirectory() doesn't walk into them (by design)

10. Added cleanup on shutdown
   - New Close() method cleans up all maps
   - Waits for active scans with 5-second timeout

11. Test timing: 11s → 15s
   - Prevents flaky tests under load

12. Added database error handling
   - Checks libraryID.Valid before scanning
   - Handles orphaned folders gracefully

NEW CODE ADDED:
- scanSemaphore chan struct{} - limits concurrent scans to 10
- activeScans map[string]bool - prevents duplicate scans
- activeScansMu sync.Mutex - protects activeScans
- performInitialScan() - scans root folders on startup
- Close() method - cleanup and graceful shutdown

ARCHITECTURAL IMPROVEMENT:
- Semaphore pattern (from Phase 1 job queue) applied at directory level
- Higher concurrency limit (10 directory scans vs 3 library scans)
- Prevents resource exhaustion while maintaining parallelism
- All map entries properly cleaned up (no memory leaks)
- Graceful shutdown with timeout

Document size: 3,130 lines (increased from 2,974 lines)
Total changes: 191 insertions, 35 deletions
 2026-03-05 12:31:32 -05:00
john-okeefe a1b6820dba docs: Restructure infrastructure plan - remove Phase 1 dependencies 
Problem:
- Phase 1 (Core Fixes Using Job Queue) conflicted with Phase 0.5
- Phase 1 added JobTypeSetFolders and processSetFoldersJob
- Phase 0.5 makes folder configuration automatic via database
- Phase 1's SetFolders() job queue approach is obsolete

Changes Made:
1. Removed Phase 1 entirely (636 lines deleted)
   - Removed JobTypeSetFolders job type
   - Removed processSetFoldersJob handler
   - Removed test isolation fixes (to be added elsewhere if needed)
   - Removed default value fixes (to be added elsewhere if needed)

2. Renumbered all subsequent phases:
   - Phase 2 (Job Queue Expansion) → Phase 1
   - Phase 3 (WebSocket Scan Progress) → Phase 2
   - Phase 4 (Caching and Monitoring) → Phase 3
   - Phase 5 (Job Queue Enhancements) → Phase 4

3. Updated all step numbers:
   - All steps renumbered to match new phase numbers
   - Step 2.x → Step 1.x, Step 3.x → Step 2.x, etc.

4. Updated job type counts:
   - Changed "8 async job types" to "7 async job types"
   - Removed setfolders from commit messages

5. Updated Summary section:
   - Removed Phase 1 time estimate
   - Added Phase 0.5 time estimate
   - Removed folder config from key design decisions
   - Updated Files Modified section

6. Updated references throughout:
   - All phase references updated to new numbers
   - All step references updated to match phase numbers

Rationale:
Phase 0.5's directory-based watching approach reads folder paths
directly from the library_folders table via GetLibraryFolders(),
making manual SetFolders() configuration unnecessary. The watcher
automatically discovers subdirectories, so job queue-based folder
configuration is no longer needed.

Phase 0.5 structure:
- Phase 0.5: Fix fsnotify Reliability (2-3 hours)
  - Directory-based watching replaces file-based event queue
  - File stability checks prevent processing incomplete files
  - Smart event merging consolidates parent/child/sibling events
  - 10-second batch processing for efficient bulk operations

- Phase 1: Job Queue Expansion (6-8 hours)
  - 7 async job types (import, convert, thumbnails, reindex, backup, analytics, sync)
  - Job management API for CRUD operations
  - Real-time job status tracking

- Phase 2: WebSocket Scan Progress (2-3 hours)
  - Real-time progress updates via WebSocket
  - Eliminates polling for job status

- Phase 3: Caching and Monitoring (2 hours)
  - Settings cache reduces database load
  - Enhanced /health endpoint

- Phase 4: Job Queue Enhancements (4-6 hours)
  - Job persistence across restarts
  - Job history and audit trail
  - Priority queue support

Total document size: 2,974 lines (reduced from 3,620 lines)

All dependencies on removed Phase 1 functionality have been eliminated.
Job queue for other tasks (import, convert, etc.) and WebSocket
integration remain unchanged and fully compatible.
 2026-03-05 12:21:54 -05:00
john-okeefe 9e5c6d4566 docs: Update Phase 0.5 with Jellyfin and Audiobookshelf research findings 
Research Summary:
Analyzed how two mature media servers handle filesystem watching to
identify best practices for fixing Bookhoard's fsnotify reliability issues.

Jellyfin (C#/.NET) Approach:
- Uses directory-based watching with 64KB internal buffer (16x default)
- Smart event merging: consolidates parent/sibling/subpath events
- 45-second self-ignore delay for internal changes
- Per-library enable/disable via configuration
- Weakness: No file stability check, processes immediately

Audiobookshelf (Node.js) Approach:
- Custom watcher wrapper for cross-platform support
- File stability check: polls mtime every 3s until stable (up to 10min timeout!)
- 10-second batch delay for processing multiple changes together
- renameDetection for move operations
- Weakness: Complex custom implementation

Phase 0.5 Plan Updates:
1. Added file stability check (Audiobookshelf approach)
   - New waitForFileStability() function
   - Polls file mtime every 3 seconds until stable
   - 60-second timeout prevents infinite waiting
   - Prevents processing files still being copied/downloaded

2. Added smart event merging (Jellyfin approach)
   - Updated markDirectoryDirty() with consolidation logic
   - Replaces child events with parent directory events
   - Handles sibling consolidation (merges to common parent)
   - Reduces redundant scans during bulk operations

3. Changed to 10-second batch delay (Audiobookshelf approach)
   - Changed from 2-second debounce to 10-second batch
   - Processes all ready directories together
   - Better balance between responsiveness and efficiency

4. Updated MediaScanner struct
   - Added fileStability map[string]time.Time field
   - Added fileStabilityMu sync.RWMutex field

5. Added comprehensive unit tests
   - TestMarkDirectoryDirty_SmartEventMerging
   - TestWaitForFileStability_StableFile
   - TestWaitForFileStability_UnstableFile
   - TestProcessDirtyDirectories_BatchesScans

6. Added comparison table showing research insights

Benefits of Combined Approach:
- No event queue overflow (directory-based watching)
- Reliable bulk import with file stability checks
- Smart event consolidation reduces redundant scans
- 10-second batch provides good responsiveness/efficiency balance
- Delete detection via 60-second polling safety net
- Works on Docker and network mounts

Files Changed:
- COMPLETE_INFRASTRUCTURE_ENHANCEMENT_PLAN.md (23 lines added)

Research Sources:
- https://github.com/jellyfin/jellyfin
- https://github.com/advplyr/audiobookshelf
 2026-03-05 11:59:39 -05:00
john-okeefe b34f0fe156 docs: add comprehensive infrastructure enhancement plan 
Add detailed implementation plan for leveraging underutilized job queue
and WebSocket infrastructure. Key focus areas:

**Core Design Principle:**
- Job queue as concurrency control mechanism (not mutex blocking)
- Non-blocking API responses for long-running operations
- Expand job queue from 10% to 90% utilization

**Phase 1 (2-3 hours): Core Concurrency Fixes**
- Add watching atomic flag to MediaScanner (prevents duplicate WatchChanges)
- Use job queue for folder configuration instead of blocking calls
- Fix test isolation with system_settings snapshot/restore
- Add job queue serialization tests

**Phase 2 (6-8 hours): Job Queue Expansion**
- Add 7 new job types: import, convert, thumbnails, reindex, backup, analytics, sync
- Create JobsHandler with REST API endpoints
- All operations support progress tracking via callbacks

**Phase 3 (2-3 hours): WebSocket Scan Progress**
- Real-time scan progress broadcasts to user's devices
- Pass ConnectionManager to Worker for WebSocket integration
- Add user ID to Job for targeted messaging

**Phase 4 (2 hours): Caching & Monitoring**
- Redis caching for frequently accessed data
- Prometheus metrics for job queue performance

**Phase 5 (4-6 hours): Job Queue Enhancements**
- Priority queues for different job types
- Job cancellation and retry logic
- Rate limiting and backpressure handling

Total estimated time: 16-22 hours for full implementation
 2026-03-05 00:43:03 -05:00