Skip to content

Conversation

@dkeegan-figma
Copy link
Contributor

Summary

Cherry-picks Shopify's thread-safety improvement (commit 2821988). Replaces standard Ruby Set with Concurrent::Set to ensure thread-safe operations in parallel test execution.

Changes

Modified Files

  • ruby/lib/ci/queue/redis/worker.rb - Use Concurrent::Set, add reserved_tests accessor
  • ruby/lib/ci/queue/static.rb - Add reserved_tests method with Concurrent::Set
  • ruby/test/integration/minitest_redis_test.rb - Updated tests, added require
  • ruby/test/minitest/queue/build_status_recorder_test.rb - Updated test helper

The Problem

Standard Ruby Set is not thread-safe. In parallel test execution, concurrent modifications to Set objects can cause:

  • Race conditions
  • Data corruption
  • Intermittent test failures

The Solution

Concurrent::Set from the concurrent-ruby gem (already a dependency via ActiveSupport) provides:

  • Thread-safe operations
  • Lock-free implementation
  • No performance penalty for single-threaded use
# Before
require 'set'
@reserved_tests = Set.new

# After
require 'concurrent/set'
@reserved_tests = Concurrent::Set.new

Conflict Resolution

Successfully integrated with Figma's features:

  • Added @reserved_tests alongside existing @reserved_test
  • Preserved all Figma-specific methods (heartbeat, idle tracking, etc.)
  • Updated all test files to use Concurrent::Set

Benefits

  • Thread Safety: Eliminates race conditions in parallel execution
  • Reliability: More stable under high concurrency
  • Future-proof: Better foundation for parallel test improvements
  • Zero Cost: Already depends on concurrent-ruby via ActiveSupport

Testing

  • ✅ Ruby syntax checks pass
  • ✅ Concurrent::Set functionality verified
  • ✅ All Figma-specific code preserved
  • ⏳ Full test suite requires Redis (run in CI)

Original Shopify PR

Risk Assessment

Risk Level: Low

  • No API changes
  • Concurrent::Set is drop-in replacement for Set
  • Already battle-tested in concurrent-ruby
  • Used in Shopify production

Notes

This change adds @reserved_tests (Set) alongside Figma's existing @reserved_test (single value). Both can coexist:

  • @reserved_test tracks the currently executing test
  • @reserved_tests tracks all reserved tests in the worker

cc: @Dkeegan

ci-queue depends on ActiveSupport, which depends on concurrent-ruby, so
we do have access to `Concurrent::Set`.
@dkeegan-figma dkeegan-figma marked this pull request as ready for review December 12, 2025 19:18
@dkeegan-figma dkeegan-figma merged commit cbfe7f6 into master Dec 16, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants