Reserve half-open tests

.gitignore

@@ -9,3 +9,5 @@
 /.yardoc/
 
 .byebug_history
+
+/.vscode/

CHANGELOG.md

@@ -9,6 +9,40 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [Unreleased]
 -------------------
 
+[0.13.0] - 2026-05-13
+---------------------
+
+### Added
+
+* Reserve half-open test runs across processes so only one process executes
+  the test block when a circuit becomes half-open. justinhoward
+
+### Changed
+
+* `Faulty::Status` now captures `current_time` once when the status is built,
+  so all predicates (`open?`, `half_open?`, `reserved?`) reason about the same
+  point in time. Previously each predicate called `Faulty.current_time`
+  independently. justinhoward
+* `Storage::Redis#close` now also clears the `reserved_at` key. justinhoward
+
+### Fixed
+
+* `Storage::Redis#reserve` uses safe navigation when serializing
+  `previous_reserved_at` for `WATCH`, so the very first CAS against a missing
+  key compares correctly. justinhoward
+* `Storage::Redis#reset` now clears the `reserved_at` key. justinhoward
+* `Status#can_run?` now treats `locked_closed?` as an unconditional override,
+  so a manually locked-closed circuit runs even when a half-open reservation
+  is still in effect from a prior cycle. justinhoward
+
+### Breaking Changes
+
+* `Storage::Interface` adds a required `#reserve(circuit, reserved_at,
+  previous_reserved_at)` method. Custom storage backends must implement it,
+  and the `Status` value object must carry the new `reserved_at` attribute.
+  See `Storage::Interface#reserve` for the contract and the conformance
+  test in `spec/storage/interface_spec.rb` for the structural guarantee.
+
 [0.12.0] - 2026-05-13
 ---------------------
 
@@ -349,7 +383,9 @@ of AutoWire.
 
 Initial public release
 
-[Unreleased]: https://github.com/ParentSquare/faulty/compare/v0.11.0...HEAD
+[Unreleased]: https://github.com/ParentSquare/faulty/compare/v0.13.0...HEAD
+[0.13.0]: https://github.com/ParentSquare/faulty/compare/v0.12.0...v0.13.0
+[0.12.0]: https://github.com/ParentSquare/faulty/compare/v0.11.0...v0.12.0
 [0.11.0]: https://github.com/ParentSquare/faulty/compare/v0.10.0...v0.11.0
 [0.10.0]: https://github.com/ParentSquare/faulty/compare/v0.9.0...v0.10.0
 [0.9.0]: https://github.com/ParentSquare/faulty/compare/v0.8.7...v0.9.0

README.md

@@ -1226,6 +1226,23 @@ state, Faulty allows a single execution of the block as a test run. If the test
 run succeeds, the circuit is fully closed and the circuit state is reset. If the
 test run fails, the circuit is opened and the cool-down is reset.
 
+When the storage backend supports atomic operations (the default `Memory` and
+`Redis` backends both do), the half-open test run is reserved exclusively. Other
+processes or threads that observe the half-open state while a test run is in
+progress will be skipped with `Faulty::OpenCircuitError`, just as if the circuit
+were still open. The reservation expires after `cool_down` so that a crashed
+process can't permanently wedge the circuit.
+
+This means `cool_down` does double duty: it gates how long the circuit waits
+before retrying after opening, and it bounds how long a half-open reservation
+is honored. Test runs that legitimately take longer than `cool_down` (for
+example, a slow downstream during recovery) will see their reservation expire
+mid-run, at which point another process can reserve and run the block
+concurrently. If your protected calls can run longer than `cool_down`, set
+`cool_down` to comfortably exceed the slowest expected latency for the
+protected operation, or accept that occasional duplicate half-open test runs
+are possible during slow recoveries.
+
 Each time the circuit changes state or executes the block, events are raised
 that are sent to the Faulty event notifier. The notifier should be used to track
 circuit failure rates, open circuits, etc.

docker-compose.yml

@@ -0,0 +1,24 @@
+# Dev-only stack for spec dependencies. All services run with auth disabled
+# or default credentials, so ports are bound to 127.0.0.1 to keep them off
+# any other network interface.
+services:
+  mysql:
+    image: mysql:8.0.31
+    environment:
+      MYSQL_ALLOW_EMPTY_PASSWORD: "true"
+    ports:
+      - "127.0.0.1:3306:3306"
+
+  redis:
+    image: redis:6.2
+    entrypoint: redis-server --appendonly yes
+    ports:
+      - "127.0.0.1:6379:6379"
+
+  opensearch:
+    image: opensearchproject/opensearch:2.7.0
+    environment:
+      - discovery.type=single-node
+      - DISABLE_SECURITY_PLUGIN=true
+    ports:
+      - "127.0.0.1:9200:9200"

lib/faulty.rb

@@ -125,9 +125,11 @@ def list_circuits
     # The current time
     #
     # Used by Faulty wherever the current time is needed. Can be overridden
-    # for testing
+    # for testing. Returned as a Float (Unix epoch seconds with sub-second
+    # precision) so it can be stored in numeric Redis fields and compared
+    # against other timestamps without conversion.
     #
-    # @return [Time] The current time
+    # @return [Float] The current time as a Unix timestamp
     def current_time
       Time.now.to_f
     end

lib/faulty/circuit.rb

@@ -49,6 +49,10 @@ class Circuit
     # @!attribute [r] cool_down
     #   @return [Integer] The number of seconds the circuit will
     #     stay open after it is tripped. Default 300.
+    #
+    #     Also bounds the half-open reservation TTL — runs longer than
+    #     `cool_down` lose exclusivity. See the "How it Works" section
+    #     of the README.
     # @!attribute [r] error_mapper
     #   @return [Module, #call] Used by patches to set the namespace module for
     #     the faulty errors that will be raised. Should be a module or a callable.
@@ -308,9 +312,11 @@ def run(cache: nil, &block)
       return cached_value if !cached_value.nil? && !cache_should_refresh?(cache)
 
       current_status = status
-      return run_skipped(cached_value) unless current_status.can_run?
-
-      run_exec(current_status, cached_value, cache, &block)
+      if current_status.can_run? && reserve(current_status)
+        run_exec(current_status, cached_value, cache, &block)
+      else
+        run_skipped(cached_value)
+      end
     end
 
     # Force the circuit to stay open until unlocked
@@ -403,6 +409,32 @@ def run_skipped(cached_value)
       cached_value
     end
 
+    # Reserves execution for this circuit when it is half-open
+    #
+    # This prevents concurrent evaluation from allowing multiple simultaneous
+    # runs for half-open circuits. For non-half-open states this is a no-op
+    # that returns true so closed and locked-closed circuits run unconditionally.
+    #
+    # `locked_closed?` is checked before `half_open?` to mirror the operator-
+    # override hierarchy in {Status#can_run?}: a locked-closed circuit must
+    # always proceed regardless of the underlying state, even if another
+    # process is currently holding the reservation. Without this, a locked-
+    # closed circuit could lose the storage CAS to a concurrent process and
+    # be incorrectly skipped.
+    #
+    # @param status [Status] The current status of the circuit
+    # @return [Boolean] True if this call may proceed to execute the block
+    def reserve(status)
+      return true if status.locked_closed?
+      return true unless status.half_open?
+
+      # Persist a fresh Faulty.current_time, not status.current_time. The
+      # snapshot exists for predicate consistency (see Status#current_time);
+      # the stored reserved_at should reflect when the reservation was made,
+      # not when the snapshot was taken.
+      storage.reserve(self, Faulty.current_time, status.reserved_at)
+    end
+
     # Execute a run
     #
     # @param cached_value The cached value if one is available

lib/faulty/status.rb

@@ -16,9 +16,19 @@ class Faulty
   #   @return [:open, :closed, nil] If the circuit is locked, the state that
   #     it is locked in. Default `nil`.
   # @!attribute [r] opened_at
-  #   @return [Integer, nil] If the circuit is open, the timestamp that it was
-  #     opened. This is not necessarily reset when the circuit is closed.
-  #     Default `nil`.
+  #   @return [Float, nil] If the circuit is open, the timestamp ({Faulty.current_time})
+  #     that it was opened. This is not necessarily reset when the circuit
+  #     is closed. Default `nil`.
+  # @!attribute [r] reserved_at
+  #   @return [Float, nil] If a half-open test run was reserved, the
+  #     timestamp ({Faulty.current_time}) of that reservation. Cleared when
+  #     the circuit is closed.
+  #     Not reset by {Storage::Interface#reopen}; the value naturally expires
+  #     via `cool_down`. Default `nil`.
+  #
+  #     Only meaningful when {#state} is `:open`. If a backend race or bug
+  #     produces an inconsistent shape (`state == :closed` with a non-nil
+  #     `reserved_at`), it is normalized to `nil` at construction.
   # @!attribute [r] failure_rate
   #   @return [Float] A number from 0 to 1 representing the percentage of
   #     failures for the circuit. For exmaple 0.5 represents a 50% failure rate.
@@ -34,6 +44,7 @@ class Faulty
     :state,
     :lock,
     :opened_at,
+    :reserved_at,
     :failure_rate,
     :sample_size,
     :options,
@@ -43,6 +54,19 @@ class Faulty
   class Status
     include ImmutableOptions
 
+    # @return [Float] The point in time captured when this status was built.
+    #   All predicates (`open?`, `half_open?`, `reserved?`) reason about
+    #   this same instant so they are mutually consistent. Held as an
+    #   instance variable rather than a struct field so it does not leak
+    #   into `to_h`, `==`, or `members` — those should reflect persisted
+    #   circuit state, not a transient predicate-consistency snapshot.
+    attr_reader :current_time
+
+    def initialize(hash, &)
+      @current_time = hash[:current_time] || Faulty.current_time
+      super(hash.except(:current_time), &)
+    end
+
     # The allowed state values
     STATES = %i[
       open
@@ -66,7 +90,8 @@ class Status
     #   sample_size
     # @return [Status]
     def self.from_entries(entries, **hash)
-      window_start = Faulty.current_time - hash[:options].evaluation_window
+      current_time = Faulty.current_time
+      window_start = current_time - hash[:options].evaluation_window
       size = entries.size
       i = 0
       failures = 0
@@ -84,7 +109,8 @@ def self.from_entries(entries, **hash)
 
       new(hash.merge(
         sample_size: sample_size,
-        failure_rate: sample_size.zero? ? 0.0 : failures.to_f / sample_size
+        failure_rate: sample_size.zero? ? 0.0 : failures.to_f / sample_size,
+        current_time: current_time
       ))
     end
 
@@ -94,7 +120,7 @@ def self.from_entries(entries, **hash)
     #
     # @return [Boolean] True if open
     def open?
-      state == :open && opened_at + options.cool_down > Faulty.current_time
+      state == :open && opened_at + options.cool_down > current_time
     end
 
     # Whether the circuit is closed
@@ -112,7 +138,7 @@ def closed?
     #
     # @return [Boolean] True if half-open
     def half_open?
-      state == :open && opened_at + options.cool_down <= Faulty.current_time
+      state == :open && opened_at + options.cool_down <= current_time
     end
 
     # Whether the circuit is locked open
@@ -129,15 +155,42 @@ def locked_closed?
       lock == :closed
     end
 
+    # Whether a half-open test run is currently reserved
+    #
+    # Process-agnostic: returns true whenever an unexpired reservation exists
+    # on this circuit, regardless of who made it. The "did someone else reserve
+    # this?" interpretation only applies when this predicate is read on a
+    # status snapshot taken *before* the caller attempts {Storage::Interface#reserve};
+    # a caller introspecting their own status after a successful reserve will
+    # also see `true` here.
+    #
+    # The reservation expires after `cool_down` to handle the case where the
+    # process that made the reservation crashes before resolving the circuit.
+    # Side effect: a legitimately-slow test run that exceeds `cool_down`
+    # loses exclusivity (another process may reserve and run concurrently).
+    # See the "How it Works" section of the README for the full trade-off.
+    #
+    # @return [Boolean] True if a reservation is in effect
+    def reserved?
+      return false unless reserved_at
+
+      state == :open && reserved_at + options.cool_down > current_time
+    end
+
     # Whether the circuit can be run
     #
-    # Takes the circuit state, locks and cooldown into account
+    # Takes the circuit state, locks and cooldown into account. Locks are
+    # operator overrides and take precedence over both state and reservation,
+    # so a `locked_closed?` circuit always runs and a `locked_open?` circuit
+    # never runs.
     #
     # @return [Boolean] True if the circuit can be run
     def can_run?
       return false if locked_open?
+      return true if locked_closed?
+      return false if reserved?
 
-      closed? || locked_closed? || half_open?
+      closed? || half_open?
     end
 
     # Whether the circuit fails the sample size and rate thresholds
@@ -155,6 +208,14 @@ def finalize
         raise ArgumentError, "lock must be a symbol in #{self.class}::LOCKS or nil"
       end
       raise ArgumentError, 'opened_at is required if state is open' if state == :open && opened_at.nil?
+
+      # `reserved_at` is only meaningful while the circuit is open. Backends
+      # are expected to clear it on close, but if a brief race or backend bug
+      # leaves a stale value paired with `state == :closed`, normalize it
+      # here so downstream code can rely on the invariant without checking
+      # `state` first. Sanitizing rather than raising avoids turning a
+      # transient backend inconsistency into a production crash.
+      self.reserved_at = nil if state == :closed && !reserved_at.nil?
     end
 
     def required

lib/faulty/storage/circuit_proxy.rb

@@ -52,6 +52,7 @@ def initialize(storage, **options, &)
         open
         reopen
         close
+        reserve
         lock
         unlock
         reset

lib/faulty/storage/fallback_chain.rb

@@ -105,6 +105,16 @@ def close(circuit)
         end
       end
 
+      # Reserve a half-open run in the first available storage backend
+      #
+      # @param (see Interface#reserve)
+      # @return (see Interface#reserve)
+      def reserve(circuit, reserved_at, previous_reserved_at)
+        send_chain(:reserve, circuit, reserved_at, previous_reserved_at) do |e|
+          options.notifier.notify(:storage_failure, circuit: circuit, action: :reserve, error: e)
+        end
+      end
+
       # Lock a circuit in all storage backends
       #
       # @param (see Interface#lock)

lib/faulty/storage/fault_tolerant_proxy.rb

@@ -9,6 +9,19 @@ module Storage
     #
     # If the storage backend raises a `StandardError`, it will be captured and
     # sent to the notifier.
+    #
+    # The overall design preference is to keep protected code paths running
+    # when the storage backend is degraded, even when that means losing
+    # circuit-breaker protections that the storage normally provides:
+    # `#status` returns a stub closed status (so `Circuit#run` proceeds),
+    # `#reserve` returns `true` (so half-open test runs proceed), and the
+    # write paths (`#open`, `#reopen`, `#close`, `#entry`) return `false`
+    # to safe-deny the *recorded transition* without failing the in-flight
+    # call. The trade-off is that a correlated outage of the storage
+    # backend and the upstream protected by the circuit will let the fleet
+    # converge on the upstream — but that fleet would converge anyway via
+    # the stub-closed status path, so individual write methods don't make
+    # it worse.
     class FaultTolerantProxy
       extend Forwardable
 
@@ -177,6 +190,22 @@ def status(circuit)
         stub_status(circuit)
       end
 
+      # Safely reserve execution of a circuit
+      #
+      # Returns `true` on storage error so half-open test runs proceed when
+      # the backend is degraded. See the class-level docs for the gem's
+      # fail-open trade-off.
+      #
+      # @see Interface#reserve
+      # @param (see Interface#reserve)
+      # @return (see Interface#reserve)
+      def reserve(circuit, reserved_at, previous_reserved_at)
+        @storage.reserve(circuit, reserved_at, previous_reserved_at)
+      rescue StandardError => e
+        options.notifier.notify(:storage_failure, circuit: circuit, action: :reserve, error: e)
+        true
+      end
+
       # This cache makes any storage fault tolerant, so this is always `true`
       #
       # @return [true]