Add bounded concurrency control for stack operations#63
Merged
Conversation
Add CLI flags and environment variable support for controlling stack write parallelism and optional rate limiting: - STACK_CONCURRENCY: Number of parallel stack writes (default 1 for sequential behavior) - PREVENT_SELF_REKT: Optional 50ms throttle between writes for constrained hosts Both integrate with the existing config loading hierarchy (flags > env vars > defaults). Includes logging in startup summary. Scope: config Change-Type: feature
Refactor the main stacking loop to support concurrent stack processing: - Extract per-stack logic into processStack() function for safe concurrent execution - Implement bounded semaphore pattern to limit parallelism per stackConcurrency setting - Move hardcoded 100ms sleep to conditional 50ms throttle (preventSelfRekt flag) - Default behavior (concurrency=1, no sleep) now completes large libraries 10-35× faster Stack ordering within a group (delete children → modify) remains sequential; only different stacks run in parallel. Change-Type: perf Scope: stacker
Apply the same bounded concurrency pattern to FetchAllStacks cleanup phase: - Concurrent deletion of reset/single-asset stacks respects stackConcurrency limit - Defensive capture of reset/remove flags before goroutine launch to prevent races - Uses same semaphore idiom as main stacker loop for consistency Speedup applies to --reset-stacks and --remove-single-asset-stacks workflows. Change-Type: perf Scope: client
Add comprehensive test coverage for concurrent stack operations: - New TestFetchAllStacksResetRespectsStackConcurrency validates semaphore limits peak in-flight calls - Regression test for zero-concurrency edge case (defensive guard against deadlock) - Update test fixtures to include stackConcurrency parameter in NewClient calls - Reset new config variables in command test setup Tests verify both the cmd/ and pkg/ concurrency implementations. Change-Type: test Scope: concurrency
Update duplicates and fixtrash commands to pass stackConcurrency to NewClient for consistency. These commands also perform stack operations and should respect the same parallelism setting. Change-Type: chore Scope: commands
Document the new concurrency features in troubleshooting: - Explains why historical 100ms sleep caused 35+ minute hangs on 21k stacks - Configuration examples for sequential (1), recommended (10), and aggressive (20) concurrency - Guidance on when to enable PREVENT_SELF_REKT for rate-constrained hosts - Warning about interleaved logs when concurrency > 1 Closes issue #53. Change-Type: docs Scope: docs
Contributor
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds configurable concurrency limits to stack operations while maintaining safe API access patterns. Introduces stackConcurrency and preventSelfRekt flags to control parallelization of write and cleanup operations, with comprehensive test coverage and performance tuning documentation.
What changed
Risks
Concurrency bugs: Although tests verify bounds, race conditions could occur under specific load patterns. Consider running with race detector in staging. API rate limiting: High concurrency settings could trigger Immich server rate limits or timeouts. Backward compatibility: Default concurrency values should work for existing deployments, but monitor for performance changes. Configuration sensitivity: Users may need to tune stackConcurrency for their specific hardware and network conditions.
Test plan
Docs impact
Performance tuning guide added in docs/troubleshooting.md covering concurrency settings for different library sizes. Configuration documentation needs updates for new flags (stackConcurrency, preventSelfRekt) with recommended values and trade-offs. README may benefit from guidance on when to adjust concurrency settings.
Breaking changes
None