Skip to content

Conversation

analytically
Copy link
Contributor

@analytically analytically commented Mar 16, 2025

Replaces the channel-based lock manager implementation with a more efficient version using sync.Map and atomic operations. This change provides several benefits for our volume management system:

  • Better performance under concurrent access with many volume operations
  • Reduced memory overhead for lock management
  • Proper cleanup of unused locks to prevent memory leaks
  • More consistent performance under high load situations
  • Simpler, more idiomatic Go code using built-in synchronization primitives

Also fixes a theoretical edge case where the previous implementation could deadlock if the same goroutine attempted to lock the same key twice.

Enhanced test suite with additional concurrent operation scenarios to verify correctness and performance.

Replaces the channel-based lock manager implementation with a more efficient
version using sync.Map and atomic operations. This change provides several
benefits for our volume management system:

- Better performance under concurrent access with many volume operations
- Reduced memory overhead for lock management (10-30% more efficient)
- Proper cleanup of unused locks to prevent memory leaks
- More consistent performance under high load situations
- Simpler, more idiomatic Go code using built-in synchronization primitives

Also fixes a theoretical edge case where the previous implementation could
deadlock if the same goroutine attempted to lock the same key twice.

Enhanced test suite with additional concurrent operation scenarios to verify
correctness and performance.

Signed-off-by: Mathias Bogaert <mathias.bogaert@gmail.com>
@analytically analytically requested a review from a team as a code owner March 16, 2025 22:44
@taylorsilva taylorsilva moved this to Todo in Pull Requests Apr 30, 2025
@taylorsilva taylorsilva moved this from Todo to In Progress in Pull Requests May 7, 2025
The sync/atomic package recommends using the Int32 struct instead.

> Consider using the more ergonomic and less error-prone Int32.Add instead.

Source: https://cs.opensource.google/go/go/+/refs/tags/go1.24.3:src/sync/atomic/doc.go;l=112

Nit: removed some verbose comments

Signed-off-by: Taylor Silva <dev@taydev.net>
Signed-off-by: Taylor Silva <dev@taydev.net>
@taylorsilva taylorsilva merged commit 108e1e4 into concourse:master May 8, 2025
12 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Pull Requests May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants