From ca6f656d59948c41b16b69d30ba97024b5a0888f Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 9 Oct 2026 15:17:23 -0400 Subject: [PATCH 1/4] Set `code` on every `perform.hot_cell` event The event left `code` nil on success and unset when an exception escaped the call, so `HotCell::LogSubscriber` and `yabeda-hotcell` each derived the call's result, and `yabeda-hotcell` counted an interrupted call as `ok` with a `perform` of 0 seconds. Set `code` to `ok` on success and to `interrupted` when an exception interrupts the call, and have both subscribers record it as given. [Fix #93] --- docs/observability.md | 9 ++++----- docs/request-lifecycle.md | 3 ++- hotcell-client/lib/hot_cell/client.rb | 10 ++++++++-- hotcell-client/lib/hot_cell/log_subscriber.rb | 2 +- hotcell-client/test/classification_test.rb | 17 +++++++++++++++-- hotcell-client/test/log_subscriber_test.rb | 7 +++---- yabeda-hotcell/lib/yabeda/hot_cell.rb | 9 +++++---- yabeda-hotcell/test/perform_test.rb | 18 ++++++++++++++++++ 8 files changed, 56 insertions(+), 19 deletions(-) diff --git a/docs/observability.md b/docs/observability.md index bf35fd1..dc11a55 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -80,7 +80,7 @@ The payload has the following keys: | --- | --- | | `operation` | The routing name. | | `cell` | The registered cell name. | -| `code` | The failure's code. `nil` on success. | +| `code` | `ok` on success, the failure's code on failure, or `interrupted` when an exception interrupted the call, such as the application's own request timeout. | | `cause` | The cause of a `killed` failure. | | `signal` | The signal that ended the worker, if any. | | `stderr` | The tail of what a dying worker wrote to file descriptor 2. Write it to a log field and nowhere else: a tool wrote it while it processed a hostile file. | @@ -106,8 +106,7 @@ In a Rails application, `HotCell::LogSubscriber` writes one `info` line for each ``` For a failed call, the line adds `cause` and `stderr` when they exist. For a call that an exception -interrupted, such as the application's own request timeout, the line has the exception's class in place -of the code. +interrupted, the code is `interrupted` and the line adds the exception's class. To turn the line off, call `HotCell::LogSubscriber.detach_from :hot_cell` in an initializer. @@ -134,8 +133,8 @@ The metrics are in the `hotcell` group: | Metric | Type | Tags | Description | | --- | --- | --- | --- | -| `requests` | counter | `cell`, `operation`, `code`, `cause` | Each call. `code` is `ok` on success, and `cause` is empty when there's none. | -| `perform` | histogram | `cell`, `operation` | Seconds that the cell spent in `perform`. A call whose response has no `perform_ms`, such as `capacity`, `unavailable` or `timeout`, records nothing. | +| `requests` | counter | `cell`, `operation`, `code`, `cause` | Each call, by the event's `code`. `cause` is empty when there's none. | +| `perform` | histogram | `cell`, `operation` | Seconds that the cell spent in `perform`. A call with no `perform_ms`, such as `capacity`, `unavailable`, `timeout` or `interrupted`, records nothing. | | `up` | gauge | `cell` | 1 when the local cell answers its control socket, otherwise 0. | | `running` | gauge | `cell` | Workers busy right now. | | `queued` | gauge | `cell` | Connections waiting for a worker. | diff --git a/docs/request-lifecycle.md b/docs/request-lifecycle.md index d02d860..0111a66 100644 --- a/docs/request-lifecycle.md +++ b/docs/request-lifecycle.md @@ -56,5 +56,6 @@ sequenceDiagram group. The supervisor is then the only process that holds the connection, and it answers `killed` with the cause. 8. The client raises the exception class that is registered for the failure's side of the permanent - split. On success and on failure, the client publishes a `perform.hot_cell` notification. See + split. On success, on failure, and when an exception interrupts the call, the client publishes a + `perform.hot_cell` notification. See [Response codes](codes.md) and [Observability](observability.md#per-call-notification). diff --git a/hotcell-client/lib/hot_cell/client.rb b/hotcell-client/lib/hot_cell/client.rb index b9e3e54..031f6fa 100644 --- a/hotcell-client/lib/hot_cell/client.rb +++ b/hotcell-client/lib/hot_cell/client.rb @@ -106,6 +106,11 @@ def perform_in_hotcell(inputs, outputs, payload = {}) ActiveSupport::Notifications.instrument "perform.hot_cell", operation: self.class.operation, cell: cell.name do |event| response = verify_output(cell.transport.call(cell, line, descriptors), outputs) publish event, cell, response, inputs, outputs + # The transport answers its own failures with a code, so what lands here is the caller's own exception, + # such as its request timeout, and the call has no answer to describe. + rescue Exception + event[:code] = "interrupted" + raise end raise_for response, cell @@ -160,7 +165,8 @@ def verify_output(response, outputs) # `code` belongs on the event rather than only on an exception. A caller configured to treat # `unreadable` as data would otherwise make those requests invisible, and unreadable rates are exactly # what you want to watch after a library upgrade. `capacity` matters for the same reason: without its - # rate you cannot size a worker pool. + # rate you cannot size a worker pool. It is `ok` on success, so no subscriber reads success from a + # missing code. # # The cause, signal and verdict go with it, because the code alone cannot classify a kill: `killed` # is permanent for fsize and memory and transient for deadline and crashed. A subscriber with only @@ -178,7 +184,7 @@ def verify_output(response, outputs) def publish(event, cell, response, inputs, outputs) failure = response.failure - event[:code] = failure&.code + event[:code] = failure ? failure.code : "ok" event[:cause] = failure&.cause event[:signal] = failure&.signal event[:stderr] = failure&.stderr diff --git a/hotcell-client/lib/hot_cell/log_subscriber.rb b/hotcell-client/lib/hot_cell/log_subscriber.rb index ffdd28e..f8bde32 100644 --- a/hotcell-client/lib/hot_cell/log_subscriber.rb +++ b/hotcell-client/lib/hot_cell/log_subscriber.rb @@ -28,7 +28,7 @@ def perform(event) " HotCell (#{duration_ms}ms) " + JSON.generate({ cell: payload[:cell], operation: payload[:operation], - code: payload[:code] || ("ok" unless payload[:exception]), + code: payload[:code], exception: payload[:exception]&.first, cause: payload[:cause], perform_ms: payload[:perform_ms], diff --git a/hotcell-client/test/classification_test.rb b/hotcell-client/test/classification_test.rb index 695f911..7f4496b 100644 --- a/hotcell-client/test/classification_test.rb +++ b/hotcell-client/test/classification_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "test_helper" +require "timeout" # Every failure a client can produce, classified. The transport is a seam, so this drives each verdict # straight through it rather than arranging a cell to produce it. @@ -180,12 +181,12 @@ def test_the_event_carries_a_transient_verdict_for_a_deadline_kill assert_equal false, event[:permanent] end - def test_the_event_carries_no_code_on_success + def test_the_event_carries_code_ok_on_success register_with HotCell::Response.ok(result: {}, timing: { perform_ms: 12, operation_ms: 9 }) event = events_for { Anything.perform_in_hotcell [], [], {} }.first.payload - assert_nil event[:code] + assert_equal "ok", event[:code] assert_nil event[:cause] assert_nil event[:signal] assert_nil event[:stderr] @@ -194,6 +195,18 @@ def test_the_event_carries_no_code_on_success assert_equal({ perform_ms: 12, operation_ms: 9 }, event[:timing]) end + def test_the_event_carries_code_interrupted_when_an_exception_escapes_the_call + interruption = Timeout::Error.new("the request ran out of time") + register_with -> { raise interruption } + + event = events_for do + assert_same interruption, assert_raises(Timeout::Error) { Anything.perform_in_hotcell [], [], {} } + end.first.payload + + assert_equal "interrupted", event[:code] + assert_equal [ "Timeout::Error", "the request ran out of time" ], event[:exception] + end + # The client never reconnects and never retries. A silent retry doubles a cell's load at the moment it is # least able to take it. def test_a_failure_is_sent_exactly_once diff --git a/hotcell-client/test/log_subscriber_test.rb b/hotcell-client/test/log_subscriber_test.rb index fd87a3f..d0b52c3 100644 --- a/hotcell-client/test/log_subscriber_test.rb +++ b/hotcell-client/test/log_subscriber_test.rb @@ -60,9 +60,8 @@ def test_a_failed_call_logs_its_code_its_cause_and_its_stderr assert_match FORGERY, fields["stderr"] end - # An exception that escapes the call, such as the application's own request timeout, still fires the event, - # but before the client has recorded any verdict. - def test_a_call_interrupted_by_an_exception_logs_the_exception_rather_than_ok + # An exception that escapes the call, such as the application's own request timeout, still fires the event. + def test_a_call_interrupted_by_an_exception_logs_code_interrupted_and_the_exception HotCell.root = "/nowhere" HotCell.register "test", permanent: Unprocessable, transient: TemporarilyUnavailable, transport: ->(*) { raise Timeout::Error, "the request ran out of time" } @@ -72,8 +71,8 @@ def test_a_call_interrupted_by_an_exception_logs_the_exception_rather_than_ok fields = logged_fields assert_equal "test", fields["cell"] assert_equal "test.uppercase", fields["operation"] + assert_equal "interrupted", fields["code"] assert_equal "Timeout::Error", fields["exception"] - assert_nil fields["code"] end class Uppercase < HotCell::Client diff --git a/yabeda-hotcell/lib/yabeda/hot_cell.rb b/yabeda-hotcell/lib/yabeda/hot_cell.rb index b8cc0fb..091f662 100644 --- a/yabeda-hotcell/lib/yabeda/hot_cell.rb +++ b/yabeda-hotcell/lib/yabeda/hot_cell.rb @@ -70,14 +70,15 @@ def self.subscribe_to_performs # A failed call is not reported from here: the raise the caller sees already reaches the error reporter. def self.record_perform(event) labels = { cell: event.payload[:cell], operation: event.payload[:operation] } + code = event.payload[:code] # Empty rather than absent, because a label that is sometimes missing is a separate series in Prometheus # and a query by code would silently split. - Yabeda.hotcell.requests.increment(labels.merge(code: event.payload[:code] || "ok", - cause: event.payload[:cause].to_s)) + Yabeda.hotcell.requests.increment(labels.merge(code: code, cause: event.payload[:cause].to_s)) - # The response has no perform_ms when the cell refuses the call with `capacity`, or when the client fails - # the call itself with `unavailable` or `timeout`. Recording 0 would pull down the low percentiles. + # The event has no perform_ms when the cell refuses the call with `capacity`, when the client fails the + # call itself with `unavailable` or `timeout`, or when an exception interrupts the call. Recording 0 + # would pull down the low percentiles. perform_ms = event.payload[:perform_ms] Yabeda.hotcell.perform.measure(labels, perform_ms / 1000.0) if perform_ms end diff --git a/yabeda-hotcell/test/perform_test.rb b/yabeda-hotcell/test/perform_test.rb index edb893e..919b7fe 100644 --- a/yabeda-hotcell/test/perform_test.rb +++ b/yabeda-hotcell/test/perform_test.rb @@ -39,6 +39,15 @@ def test_counts_a_kill_under_its_cause assert_equal 1, requests(code: "killed", cause: "fsize") end + def test_counts_a_call_interrupted_by_an_exception_under_its_own_code_and_measures_nothing + answer_by_raising + + assert_raises(RuntimeError) { Anything.perform_in_hotcell [], [], {} } + + assert_equal({ { **labels, code: "interrupted", cause: "" } => 1 }, all_requests) + assert_nil perform_seconds + end + # A subscriber raises into whoever called instrument, so a bug here would otherwise arrive as a failed call. def test_a_metrics_bug_arrives_as_a_report_rather_than_as_a_failed_call ActiveSupport::Notifications.instrument("perform.hot_cell", cell: "test", perform_ms: "not a number") { } @@ -52,6 +61,11 @@ def answer(response) register transport: CannedTransport.new(response) end + def answer_by_raising + HotCell.root = "/nowhere" + register transport: BrokenTransport.new + end + def failed(code:, cause: nil) HotCell::Response.failed HotCell::Failure.new(code: code, cause: cause, message: "no"), timing: { perform_ms: 1 } end @@ -60,6 +74,10 @@ def requests(**tags) Yabeda::TestAdapter.instance.counters[Yabeda.hotcell.requests][{ **labels, **tags }] end + def all_requests + Yabeda::TestAdapter.instance.counters[Yabeda.hotcell.requests] + end + def perform_seconds Yabeda::TestAdapter.instance.histograms[Yabeda.hotcell.perform][labels] end From 223c08a745274ae564572cf89a6e5aea9747c9ee Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 9 Oct 2026 15:18:05 -0400 Subject: [PATCH 2/4] Rename `Verdict` to `Failure::Exception` `Verdict` was a third name for a failure, beside `Failure` and the wire's `error`. Rename `HotCell::Verdict` to `HotCell::Failure::Exception`, and `HotCell::Client::Legacy::Verdict` to `HotCell::Client::Legacy::Failure::Exception`. `hot_cell_failure` is unchanged. Inside `HotCell::Failure`, a bare `Exception` now names the new module, so `Failure.for` checks for `::Exception`. --- docs/client-api.md | 2 +- docs/client-legacy.md | 2 +- docs/codes.md | 5 ++-- .../lib/hot_cell/client/legacy.rb | 24 +++++++++---------- hotcell-client-legacy/test/transport_test.rb | 4 ++-- hotcell-client/lib/hot_cell/client.rb | 2 +- hotcell-client/lib/hot_cell/failures.rb | 20 +++++++++------- hotcell-client/test/classification_test.rb | 16 +++++++++++-- hotcell-core/lib/hot_cell/failure.rb | 5 ++-- 9 files changed, 48 insertions(+), 32 deletions(-) diff --git a/docs/client-api.md b/docs/client-api.md index a790018..cea9523 100644 --- a/docs/client-api.md +++ b/docs/client-api.md @@ -117,7 +117,7 @@ publishes a `perform.hot_cell` notification. See [Observability](observability.md#per-call-notification). The exception carries the failure. Call `hot_cell_failure` on it to read the failure's `code` and `cause`, -or rescue `HotCell::Verdict` to catch it whatever classes you registered. See +or rescue `HotCell::Failure::Exception` to catch it whatever classes you registered. See [Response codes](codes.md#exception-classes). ### Errors that the client raises for a bad call diff --git a/docs/client-legacy.md b/docs/client-legacy.md index 7e4b900..ad9d9c0 100644 --- a/docs/client-legacy.md +++ b/docs/client-legacy.md @@ -59,7 +59,7 @@ end | `HotCell::Client::Legacy::PermanentFailure` | The cell marks the failure permanent. A retry fails the same way. | | `HotCell::Client::Legacy::TransientFailure` | Any other failure. A retry might succeed. | -Both classes include `HotCell::Client::Legacy::Verdict`, so you can rescue `Verdict` to catch either one. +Both classes include `HotCell::Client::Legacy::Failure::Exception`, so you can rescue `Failure::Exception` to catch either one. Call `hot_cell_failure` on the exception to read the failure, as with `hotcell-client`. See [Exception classes](codes.md#exception-classes). The failure has these attributes: diff --git a/docs/codes.md b/docs/codes.md index 576643a..f229ab5 100644 --- a/docs/codes.md +++ b/docs/codes.md @@ -147,10 +147,11 @@ the `stderr` tail in parentheses when it exists. For example, `killed: crashed ( To read the failure without parsing the message, call `hot_cell_failure` on the exception. It returns the `HotCell::Failure`, which has `code`, `cause`, `error_class`, `message`, `signal`, `stderr`, and `permanent?`. The client extends every exception that it raises for a failed call with the -`HotCell::Verdict` module, so you can also rescue `HotCell::Verdict` whatever classes you registered: +`HotCell::Failure::Exception` module, so you can also rescue `HotCell::Failure::Exception` whatever +classes you registered: ```ruby -rescue HotCell::Verdict => error +rescue HotCell::Failure::Exception => error raise unless error.hot_cell_failure.cause == HotCell::Codes::PROTECTED lock_placeholder diff --git a/hotcell-client-legacy/lib/hot_cell/client/legacy.rb b/hotcell-client-legacy/lib/hot_cell/client/legacy.rb index f8ddfba..64de94b 100644 --- a/hotcell-client-legacy/lib/hot_cell/client/legacy.rb +++ b/hotcell-client-legacy/lib/hot_cell/client/legacy.rb @@ -56,27 +56,27 @@ def to_h def to_s [ (code unless code.empty?), cause, error_class, message ].compact.join(": ") end - end - # Included in both failure classes, so `rescue Verdict` catches either and `hot_cell_failure` reads the - # failure, as with hotcell-client's HotCell::Verdict. The classes share no superclass, so rescuing one by - # name never catches the other: a permanent failure may be written down against a file forever, and a - # transient one must be retried. - module Verdict - attr_reader :hot_cell_failure + # Included in both failure classes, so `rescue Failure::Exception` catches either and + # `hot_cell_failure` reads the failure, as with hotcell-client's HotCell::Failure::Exception. The + # classes share no superclass, so rescuing one by name never catches the other: a permanent failure + # may be written down against a file forever, and a transient one must be retried. + module Exception + attr_reader :hot_cell_failure - def initialize(failure) - @hot_cell_failure = failure - super(failure.to_s) + def initialize(failure) + @hot_cell_failure = failure + super(failure.to_s) + end end end class PermanentFailure < StandardError - include Verdict + include Failure::Exception end class TransientFailure < StandardError - include Verdict + include Failure::Exception end class Timeout < StandardError; end diff --git a/hotcell-client-legacy/test/transport_test.rb b/hotcell-client-legacy/test/transport_test.rb index 114fa3f..c9ffc44 100644 --- a/hotcell-client-legacy/test/transport_test.rb +++ b/hotcell-client-legacy/test/transport_test.rb @@ -155,13 +155,13 @@ def test_a_failure_with_no_code_leaves_it_out_of_the_message end end - def test_a_verdict_can_be_rescued_as_one + def test_a_failure_exception_can_be_rescued_as_one answer = %({"v":1,"ok":false,"error":{"code":"unreadable","permanent":true,"cause":"protected"}}\n) with_peer(lambda { |connection| connection.write answer }) do |client| begin client.perform "test.echo", [], [] flunk "expected a raise" - rescue HotCell::Client::Legacy::Verdict => error + rescue HotCell::Client::Legacy::Failure::Exception => error assert_equal "protected", error.hot_cell_failure.cause end end diff --git a/hotcell-client/lib/hot_cell/client.rb b/hotcell-client/lib/hot_cell/client.rb index 031f6fa..0130d84 100644 --- a/hotcell-client/lib/hot_cell/client.rb +++ b/hotcell-client/lib/hot_cell/client.rb @@ -206,7 +206,7 @@ def raise_for(response, cell) return if response.ok? failure = response.failure - error = Verdict.wrap(cell.exception_for(failure).new(failure.to_s), failure) + error = Failure::Exception.wrap(cell.exception_for(failure).new(failure.to_s), failure) cell.report_contract_skew error if failure.code == "protocol" raise error diff --git a/hotcell-client/lib/hot_cell/failures.rb b/hotcell-client/lib/hot_cell/failures.rb index ca5d267..b52e5d8 100644 --- a/hotcell-client/lib/hot_cell/failures.rb +++ b/hotcell-client/lib/hot_cell/failures.rb @@ -22,16 +22,18 @@ class PermanentFailure < StandardError; end # failure into a permanent one. class TransientFailure < StandardError; end - # Extended onto every exception the client raises, so an application can read the failure's code, - # cause and permanence without parsing the message — and rescue HotCell::Verdict itself, whatever - # classes the registration injected. - module Verdict - attr_reader :hot_cell_failure + class Failure + # Extended onto every exception the client raises, so an application can read the failure's code, + # cause and permanence without parsing the message — and rescue HotCell::Failure::Exception itself, + # whatever classes the registration injected. + module Exception + attr_reader :hot_cell_failure - def self.wrap(error, failure) - error.extend(self) - error.instance_variable_set(:@hot_cell_failure, failure) - error + def self.wrap(error, failure) + error.extend(self) + error.instance_variable_set(:@hot_cell_failure, failure) + error + end end end end diff --git a/hotcell-client/test/classification_test.rb b/hotcell-client/test/classification_test.rb index 7f4496b..a8512d8 100644 --- a/hotcell-client/test/classification_test.rb +++ b/hotcell-client/test/classification_test.rb @@ -75,17 +75,29 @@ def test_the_raised_message_carries_enough_to_re_decide_on_later assert_match "bad PNG header", error.message end - def test_a_verdict_can_be_rescued_as_one + def test_a_failure_exception_can_be_rescued_as_one register_with failed(code: "capacity") begin Anything.perform_in_hotcell [], [], {} flunk "expected a raise" - rescue HotCell::Verdict => error + rescue HotCell::Failure::Exception => error assert_equal "capacity", error.hot_cell_failure.code end end + # `HotCell::Failure::Exception` shadows Ruby's Exception inside `HotCell::Failure`, where `Failure.for` asks + # whether its detail is one. + def test_a_socket_error_keeps_its_class_on_the_failure + HotCell.root = "/nowhere" + HotCell.register "test", permanent: Unprocessable, transient: TemporarilyUnavailable + + error = assert_raises(TemporarilyUnavailable) { Anything.perform_in_hotcell [], [], {} } + + assert_equal "unavailable", error.hot_cell_failure.code + assert_equal "Errno::ENOENT", error.hot_cell_failure.error_class + end + def test_the_raised_exception_carries_the_failure_itself register_with failed(code: "killed", cause: "deadline") diff --git a/hotcell-core/lib/hot_cell/failure.rb b/hotcell-core/lib/hot_cell/failure.rb index adb6867..7274214 100644 --- a/hotcell-core/lib/hot_cell/failure.rb +++ b/hotcell-core/lib/hot_cell/failure.rb @@ -59,9 +59,10 @@ def to_s class << self # Builds from either a message String or an Exception. An Exception has to become two wire fields, and # that rule was written out at three call sites across two gems — the worker, the supervisor's control - # answer, and the client's transport. + # answer, and the client's transport. `::Exception` because hotcell-client defines Failure::Exception, + # which a bare `Exception` here would name. def for(code, detail, cause: nil) - if detail.is_a?(Exception) + if detail.is_a?(::Exception) new code: code, cause: cause, error_class: detail.class.name, message: detail.message else new code: code, cause: cause, message: detail From b573a983e3c1399a428cb0cbb880cb8d6f3b239e Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 9 Oct 2026 15:21:18 -0400 Subject: [PATCH 3/4] Say "failure" where comments and docs said "verdict" Comments, docs and test names called a failure a "verdict", and the `requests` metric counted calls "by outcome". Use "failure", "code", "cause" and "permanent", and "answer" for what a cell sends back. Rename `Supervisor#outcome_code` to `reported_code` and `verdict_for` in `examples/load` to `code_for`. The log's `event.outcome` keeps its name because ECS defines it, and "outcome" stays where it means the result of a filesystem operation. --- .../lib/active_storage/hot_cell/client.rb | 2 +- .../hot_cell/client/analyzers/analyzing.rb | 2 +- .../test/analyzers/image_analyzer_test.rb | 2 +- .../test/test_helper.rb | 2 +- .../test/transformers/vips_test.rb | 6 ++--- .../hot_cell/server/analyzers/image/vips.rb | 2 +- .../hot_cell/server/magick_operation.rb | 4 +-- .../hot_cell/server/vips_operation.rb | 2 +- .../test/analyze_image_test.rb | 2 +- .../test/magick_transform_image_test.rb | 2 +- .../test/transform_image_test.rb | 2 +- bin/conformance | 2 +- docs/cell-settings.md | 2 +- docs/codes.md | 2 +- docs/concepts.md | 2 +- docs/conformance.md | 2 +- docs/container.md | 2 +- docs/contributing/guide.md | 6 ++--- docs/contributing/overhead.md | 2 +- docs/imagemagick.md | 2 +- docs/observability.md | 4 +-- docs/tuning.md | 12 ++++----- examples/gate | 2 +- examples/lib/battery.rb | 2 +- examples/load | 16 ++++++------ examples/operations/greedy.rb | 2 +- examples/operations/overflow.rb | 2 +- .../lib/hot_cell/client/legacy.rb | 2 +- hotcell-client/lib/hot_cell/cell.rb | 8 +++--- hotcell-client/lib/hot_cell/client.rb | 2 +- hotcell-client/test/classification_test.rb | 22 ++++++++-------- hotcell-core/lib/hot_cell/codes.rb | 4 +-- hotcell-core/lib/hot_cell/descriptors.rb | 2 +- hotcell-core/lib/hot_cell/errors.rb | 6 ++--- hotcell-core/lib/hot_cell/failure.rb | 4 +-- hotcell-core/lib/hot_cell/payload.rb | 2 +- hotcell-core/test/codes_test.rb | 10 +++---- hotcell-core/test/descriptors_test.rb | 2 +- hotcell-server/lib/hot_cell/configuration.rb | 2 +- hotcell-server/lib/hot_cell/operation.rb | 2 +- hotcell-server/lib/hot_cell/server/errors.rb | 2 +- hotcell-server/lib/hot_cell/supervisor.rb | 20 +++++++------- .../lib/hot_cell/test_operations.rb | 4 +-- hotcell-server/lib/hot_cell/timing.rb | 2 +- hotcell-server/lib/hot_cell/worker.rb | 12 ++++----- hotcell-server/test/cell_test.rb | 26 +++++++++---------- hotcell-server/test/control_test.rb | 8 +++--- hotcell-server/test/environment_test.rb | 2 +- hotcell-server/test/resource_limits_test.rb | 6 ++--- hotcell-server/test/scheduling_test.rb | 4 +-- hotcell-server/test/support/test_cell.rb | 2 +- hotcell-server/test/test_cell_test.rb | 4 +-- hotcell-server/test/worker_reports_test.rb | 2 +- hotcell-server/test/worker_test.rb | 2 +- yabeda-hotcell/lib/yabeda/hot_cell.rb | 2 +- yabeda-hotcell/test/perform_test.rb | 2 +- 56 files changed, 129 insertions(+), 129 deletions(-) diff --git a/activestorage-hotcell-client/lib/active_storage/hot_cell/client.rb b/activestorage-hotcell-client/lib/active_storage/hot_cell/client.rb index 0fd84b6..418afb6 100644 --- a/activestorage-hotcell-client/lib/active_storage/hot_cell/client.rb +++ b/activestorage-hotcell-client/lib/active_storage/hot_cell/client.rb @@ -28,7 +28,7 @@ module HotCell module Client # These four jobs declare `retry_on ActiveStorage::IntegrityError` and nothing else, and ActiveJob does # not retry by default. They should retry the transient class too: `capacity` most obviously, and every - # other transient verdict. The policy matches the one they already declare, so a cell failure and an + # other transient failure. The policy matches the one they already declare, so a cell failure and an # integrity failure back off the same way. # # Which of these classes exists depends on the Rails version, so a name that is not loaded is skipped. diff --git a/activestorage-hotcell-client/lib/active_storage/hot_cell/client/analyzers/analyzing.rb b/activestorage-hotcell-client/lib/active_storage/hot_cell/client/analyzers/analyzing.rb index f155889..0cf21b7 100644 --- a/activestorage-hotcell-client/lib/active_storage/hot_cell/client/analyzers/analyzing.rb +++ b/activestorage-hotcell-client/lib/active_storage/hot_cell/client/analyzers/analyzing.rb @@ -16,7 +16,7 @@ module Analyzers # **Which way `metadata` fails is the whole design of the method.** The built-in vips analyzer rescues # every Vips::Error and returns an empty hash, which Rails then merges with `analyzed: true` — so an # undecodable image is recorded as successfully analyzed, forever, and nothing ever re-enqueues - # AnalyzeJob. That is right for a permanent verdict and catastrophic for a transient one. + # AnalyzeJob. That is right for a permanent failure and catastrophic for a transient one. # # So a permanent failure follows the built-in behaviour and lets the blob be marked analyzed, with the # reason written to the log so it can be re-decided later against a newer library. A transient failure diff --git a/activestorage-hotcell-client/test/analyzers/image_analyzer_test.rb b/activestorage-hotcell-client/test/analyzers/image_analyzer_test.rb index b449da2..8d1a0f0 100644 --- a/activestorage-hotcell-client/test/analyzers/image_analyzer_test.rb +++ b/activestorage-hotcell-client/test/analyzers/image_analyzer_test.rb @@ -22,7 +22,7 @@ def test_it_reports_the_dimensions_exif_asks_for # The built-in vips analyzer rescues every Vips::Error and returns an empty hash, which Rails merges with # `analyzed: true` — so an undecodable image is recorded as successfully analyzed, forever, and nothing ever - # re-enqueues AnalyzeJob. That is the right answer for a permanent verdict, so it is copied deliberately. + # re-enqueues AnalyzeJob. That is the right answer for a permanent failure, so it is copied deliberately. def test_a_permanently_undecodable_image_is_marked_analyzed_the_way_rails_marks_it with_cell do assert_empty analyze("broken.png") diff --git a/activestorage-hotcell-client/test/test_helper.rb b/activestorage-hotcell-client/test/test_helper.rb index e4d860e..f2597c4 100644 --- a/activestorage-hotcell-client/test/test_helper.rb +++ b/activestorage-hotcell-client/test/test_helper.rb @@ -54,7 +54,7 @@ def with_cell end end - # For the cases about classification, where booting a cell to produce one verdict would be theatre. + # For the cases about classification, where booting a cell to produce one failure would be theatre. def with_canned_response(response) HotCell.root = "/nowhere" transport = CannedTransport.new(response) diff --git a/activestorage-hotcell-client/test/transformers/vips_test.rb b/activestorage-hotcell-client/test/transformers/vips_test.rb index eeac637..ecd0b60 100644 --- a/activestorage-hotcell-client/test/transformers/vips_test.rb +++ b/activestorage-hotcell-client/test/transformers/vips_test.rb @@ -115,9 +115,9 @@ def test_a_loader_reaches_the_cell_and_keeps_every_frame # declares Vips::Error `unreadable`, so it lands permanent. # # KNOWN DEVIATION (resolved by the planned ImageMagick operations): this is a caller bug, not a bad - # document, and Rails treats it as an unclassified job failure rather than a verdict on the blob. The + # document, and Rails treats it as an unclassified job failure rather than a permanent failure of the blob. The # `unreadable Vips::Error` rule was written for the load phase, where a Vips::Error means bad pixels; a - # bad operation name raising the same class slips through as a false permanent verdict. The real fix is an + # bad operation name raising the same class slips through as a false permanent failure. The real fix is an # ImageMagick transformer where `coalesce` is a real operation — until then an application moving from # mini_magick to vips rewrites these URLs at its own boundary. This test pins the current behavior so the # deviation is visible, not hidden. @@ -139,7 +139,7 @@ def test_an_undecodable_image_raises_the_applications_permanent_class end end - # A caller bug, not a verdict on the document: ImageProcessing refuses the name, the failure is + # A caller bug, not a permanent failure of the document: ImageProcessing refuses the name, the failure is # unclassified and therefore transient, and it stays distinguishable from an undecodable image — which # raises the permanent class and gets a placeholder. def test_an_unknown_transformation_raises_the_transient_class diff --git a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/analyzers/image/vips.rb b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/analyzers/image/vips.rb index ccbb439..2741d69 100644 --- a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/analyzers/image/vips.rb +++ b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/analyzers/image/vips.rb @@ -18,7 +18,7 @@ module Image # deliberately does not copy: it rescues every Vips::Error and returns an empty hash, which is then # merged with `analyzed: true`. An undecodable image is recorded as successfully analyzed, forever, # and nothing re-enqueues AnalyzeJob. Here an undecodable input raises Vips::Error, the cell answers - # `unreadable`, and the client decides — because only the client knows whether that verdict is safe + # `unreadable`, and the client decides — because only the client knows whether that failure is safe # to write down. class Vips < VipsOperation operation "active_storage.analyzers.image.vips" diff --git a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/magick_operation.rb b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/magick_operation.rb index 42baab0..46acb12 100644 --- a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/magick_operation.rb +++ b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/magick_operation.rb @@ -60,7 +60,7 @@ def initialize # MiniMagick::Error is how mini_magick reports a `magick` that exited non-zero — the common shape of an # input it cannot decode. MiniMagick::Invalid is an input `identify` rejects outright. - # ImageProcessing::Error is the pipeline's own verdict on the input against the requested + # ImageProcessing::Error is the pipeline's own rejection of the input against the requested # transform — a multi-layer source into a single-layer destination, for example. All three are # the input's fault rather than the operation's; unclassified, each was a transient `failed` # that never marked the blob, so the same file spent a full conversion on every request. @@ -69,7 +69,7 @@ def initialize # **Accepted risk.** A tool's output is not bounded on this path. `Operation#run_tool` caps what it # reads at 64KB and drops the rest as it arrives, because an input that makes a tool print gigabytes # of diagnostics costs this worker gigabytes of address space, takes RLIMIT_DATA with it, and arrives - # as a `memory` verdict — which is permanent, for a document whose only crime was being noisy. + # as a `memory` failure — which is permanent, for a document whose only crime was being noisy. # mini_magick reads both streams to EOF in a thread apiece and has no setting that bounds either; # `graphicsmagick`, `cli_prefix`, `cli_env`, `restricted_env`, `timeout`, `logger`, `tmpdir`, # `errors` and `warnings` are the whole list. The premise is that patching the library is worse than diff --git a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/vips_operation.rb b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/vips_operation.rb index dfaea07..ea5e607 100644 --- a/activestorage-hotcell-server/lib/active_storage/hot_cell/server/vips_operation.rb +++ b/activestorage-hotcell-server/lib/active_storage/hot_cell/server/vips_operation.rb @@ -22,7 +22,7 @@ class VipsOperation < Operation # Vips::Error is how libvips reports a file it cannot decode, which is common rather than exceptional: it # covers truncated uploads, formats this build was not compiled with, and formats deliberately refused by - # block_untrusted. ImageProcessing::Error is the pipeline's own verdict on the input against the + # block_untrusted. ImageProcessing::Error is the pipeline's own rejection of the input against the # requested transform, classified the same way. All of those are the input's fault and none is # the operation's. unreadable Vips::Error, ImageProcessing::Error diff --git a/activestorage-hotcell-server/test/analyze_image_test.rb b/activestorage-hotcell-server/test/analyze_image_test.rb index 19de3f0..e431567 100644 --- a/activestorage-hotcell-server/test/analyze_image_test.rb +++ b/activestorage-hotcell-server/test/analyze_image_test.rb @@ -53,7 +53,7 @@ def test_analysis_says_how_many_frames_there_are # The built-in vips analyzer rescues every Vips::Error and returns an empty hash, which is then merged with # `analyzed: true` — so an undecodable image is recorded as successfully analyzed, forever, and nothing - # re-enqueues AnalyzeJob. This deliberately does not copy that: the cell reports the verdict and the client + # re-enqueues AnalyzeJob. This deliberately does not copy that: the cell reports the failure and the client # decides, because only the client knows whether it is safe to write down. # Rails imposes no size limit on analysis, and neither may the cell. The input is read through its # descriptor rather than copied onto scratch, so a file far larger than the operation's file_size — which diff --git a/activestorage-hotcell-server/test/magick_transform_image_test.rb b/activestorage-hotcell-server/test/magick_transform_image_test.rb index 06813d3..5b9f7ff 100644 --- a/activestorage-hotcell-server/test/magick_transform_image_test.rb +++ b/activestorage-hotcell-server/test/magick_transform_image_test.rb @@ -51,7 +51,7 @@ def test_an_imagemagick_only_operation_runs # The production shape that surfaced this: a multi-layer source, its frames kept by # `loader: { page: nil }`, into a single-layer destination. ImageProcessing refuses the - # combination, which is a verdict on the input — unclassified it was a transient `failed` + # combination, which is a permanent failure of the input — unclassified it was a transient `failed` # that never marked the blob, so the same file spent a full conversion on every request. def test_a_multi_layer_source_into_a_single_layer_format_is_unreadable Cell.boot do |cell| diff --git a/activestorage-hotcell-server/test/transform_image_test.rb b/activestorage-hotcell-server/test/transform_image_test.rb index 6138451..d596f34 100644 --- a/activestorage-hotcell-server/test/transform_image_test.rb +++ b/activestorage-hotcell-server/test/transform_image_test.rb @@ -186,7 +186,7 @@ def test_a_format_no_saver_answers_to_is_a_vips_error # A name that is neither an ImageProcessing operation nor a Vips::Image method is refused by # ImageProcessing itself, exactly as it is under Rails on vips. It arrives `failed` — transient — so a # caller bug is never written down against the document. - def test_an_unknown_transformation_is_not_a_verdict_on_the_document + def test_an_unknown_transformation_is_not_a_permanent_failure_of_the_document Cell.boot do |cell| with_output do |destination| failure = assert_failed "failed", cell.call("active_storage.transformers.image.vips", diff --git a/bin/conformance b/bin/conformance index b66d639..67aaac0 100755 --- a/bin/conformance +++ b/bin/conformance @@ -63,7 +63,7 @@ docker volume create "$VOLUME" >/dev/null # The security flags the accessory sets, added one at a time so a negative job can leave exactly one out. # `keep NAME arg...` appends the flag unless it is the one named in DROP. No --ulimit stack: docs/container.md # forbids it — a lowered stack turns an overflow into a SIGSEGV the cell reports as killed/crashed, a transient -# verdict the caller retries against a limit that will fail it again, so the helper would measure a shape +# failure the caller retries against a limit that will fail it again, so the helper would measure a shape # operators are told not to deploy. run=(docker run --detach --name "$CELL") diff --git a/docs/cell-settings.md b/docs/cell-settings.md index 2e03634..737a595 100644 --- a/docs/cell-settings.md +++ b/docs/cell-settings.md @@ -42,7 +42,7 @@ limit. | --- | --- | --- | | `concurrency` | `4` | Workers that run at once, and the number of slots. Start at twice `cpus`. See [Settings that trade isolation](#settings-that-trade-isolation). | | `queue_size` | `8` | Connections that can wait for a worker. When `running + queued` reaches `concurrency + queue_size`, the cell answers `capacity`. To refuse instead of queueing, use `0`. | -| `queue_wait` | `10` | Seconds that a queued connection can wait before the cell answers `capacity`. A saturated cell then answers with a verdict instead of holding the caller until the caller's own timeout. | +| `queue_wait` | `10` | Seconds that a queued connection can wait before the cell answers `capacity`. A saturated cell then answers with a failure instead of holding the caller until the caller's own timeout. | | `max_requests_per_worker` | `1` | Requests that one worker serves before the cell discards it. `1` forks for each request. `:unlimited` keeps a worker for the life of the cell. A reused worker skips the copy-on-write cost of a new fork, which is a performance setting with an isolation cost. See [Settings that trade isolation](#settings-that-trade-isolation). | | `control_deadline` | `5` | Seconds that a control connection can take to send its request. | | `sweep_interval` | `10` | How often, in seconds, the supervisor looks for the directories that killed requests left behind. When it finds one, it forks a sweeper process to delete them. The sweeper runs under `deadline` like a worker, so the supervisor and the requests in flight never wait on the deletion. | diff --git a/docs/codes.md b/docs/codes.md index f229ab5..577e2b5 100644 --- a/docs/codes.md +++ b/docs/codes.md @@ -159,7 +159,7 @@ rescue HotCell::Failure::Exception => error ## What Active Storage records -A permanent verdict is irreversible only if the application records it. In the shipped Active Storage +A permanent failure is irreversible only if the application records it. In the shipped Active Storage integration, analysis records it and nothing else does. Rails persists a blob's analysis like this: diff --git a/docs/concepts.md b/docs/concepts.md index 9120a93..6258a9a 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -119,5 +119,5 @@ See [Inputs and outputs](operation-api.md#inputs-and-outputs). ## Code Every failure carries a code, and each code is permanent or transient. A permanent failure means that the -input fails this way every time, so the caller can record that verdict against the input. Every uncertain +input fails this way every time, so the caller can record that failure against the input. Every uncertain failure is transient, which means that it might succeed on a retry. See [Response codes](codes.md). diff --git a/docs/conformance.md b/docs/conformance.md index 0f82c1a..dceb8e8 100644 --- a/docs/conformance.md +++ b/docs/conformance.md @@ -27,7 +27,7 @@ bin/conformance my-cell:test 1. Boots the image with the flags in [Container flags](container.md#container-flags). 2. Mounts the example operations over `/hotcell/operations`. 3. Runs a battery of checks from a second container over a shared volume: descriptor round trips, each - kill verdict, refusal at capacity, and the isolation flags. + cause of `killed`, refusal at capacity, and the isolation flags. 4. Runs the same battery again against the image booted without one security flag at a time, and requires each run to fail at that flag's own check. diff --git a/docs/container.md b/docs/container.md index 66167ed..aeae9ff 100644 --- a/docs/container.md +++ b/docs/container.md @@ -107,7 +107,7 @@ Leave `ulimit: stack` unset. A container inherits the Docker daemon's value, nor Lowering it to 2MB buys a worker about 24MB more room, and nothing for an operation that shells out. It costs far more than that: a thread that overflows the smaller stack dies on `SIGSEGV`, and the cell reports that as `killed` with cause `crashed`, which is transient. The caller's job retries the request -against a limit that fails it again, for as long as the job keeps trying, and nothing in the verdict +against a limit that fails it again, for as long as the job keeps trying, and nothing in the failure points at the setting. Raise the cell's `memory` instead. The stack limit is a container flag rather than a `config.rb` setting because glibc reads it at exec, diff --git a/docs/contributing/guide.md b/docs/contributing/guide.md index 39fb4f7..91ec981 100644 --- a/docs/contributing/guide.md +++ b/docs/contributing/guide.md @@ -104,7 +104,7 @@ work socket answers, because `describe` and `metrics` cross the control socket i | `sleep` | block for `seconds` | the deadline kills and answers; head-of-line behavior under load | | `greedy` | allocate `megabytes` | the `memory` clamp → `killed: memory`, where enforceable | | `overflow` | write `megabytes` to the output | the `file_size` clamp → `killed: fsize` | -| `crash` | raise, or die by signal | worker death → a clean verdict and continued service | +| `crash` | raise, or die by signal | worker death → a clean failure and continued service | | `spawn` | start a grandchild that would outlive the worker | the group kill leaves nothing behind | | `probe` | whether a pid is alive, asked from inside the cell | how `spawn` is watched, in any pid namespace | | `isolation` | interfaces, root writability, scratch `noexec`, a tool's environment | the container-only checks, from the only place that can make them | @@ -147,7 +147,7 @@ published base image, so the installed scaffold is the only kind of cell image t ### `bin/load IMAGE` — how does it behave under pressure? The same operations at volume against a containerized cell. It reports throughput, latency split into -`queued_ms` against `perform_ms`, and the verdict breakdown, which is what tells saturation apart from +`queued_ms` against `perform_ms`, and the count of each code, which is what tells saturation apart from slowness. Scenarios: `echo` for a baseline, `sleep` for queueing, `greedy` and `overflow` for the resource kills, `crash` for worker death, `spawn` for orphans, and `mix` for a weighted blend. @@ -191,7 +191,7 @@ written down — `unsetenv_others: true` is proved by setting a variable, runnin that the tool never saw it. Where a control has no reachable trigger and so cannot be tested, it says so where it lives. -**A verdict that cannot be taken back needs a reason.** `permanent` means an application may write a failure +**A failure that cannot be taken back needs a reason.** `permanent` means an application may write a failure down against a blob and serve it from a cache forever, so anything unclassified is transient by default. `hotcell-core/lib/hot_cell/codes.rb` carries the argument for each code; a new code goes in that table, and a new kill reason goes in `PERMANENT_BY_CAUSE`, or it silently becomes the wrong kind of permanent. diff --git a/docs/contributing/overhead.md b/docs/contributing/overhead.md index 895885d..7bbc554 100644 --- a/docs/contributing/overhead.md +++ b/docs/contributing/overhead.md @@ -21,7 +21,7 @@ Expect the ratio to look worst on the smallest thumbnails and to stop mattering Four candidates were tested and eliminated, recorded so nobody re-runs them: -| Candidate | Verdict | +| Candidate | Finding | | --- | --- | | The container | Within noise of the native cell on two of three sources, 12ms on the largest. | | The `fork` syscall | 2.8 ms of a 20–30 ms cost. | diff --git a/docs/imagemagick.md b/docs/imagemagick.md index 031a142..f98fa0c 100644 --- a/docs/imagemagick.md +++ b/docs/imagemagick.md @@ -75,7 +75,7 @@ set a `temporary-path` policy, which overrides it. **`file_size` can't do the disk limit's job.** `file_size` is `RLIMIT_FSIZE`, a limit on each file. A layered PSD writes one cache file for each layer, and twenty 40MiB caches total 800MiB with no file near a 48MiB `file_size`. Only `MAGICK_DISK_LIMIT` bounds the sum. Where the two meet, the smaller fires first, as -an `fsize` kill or as `cache resources exhausted`. Both are permanent verdicts on the file. +an `fsize` kill or as `cache resources exhausted`. Both are permanent failures of the file. **A disk limit larger than the scratch isn't a limit.** The scratch fills first. A full scratch fails every request on the cell that needs scratch, and a write that fails inside libvips is a `Vips::Error`, which the diff --git a/docs/observability.md b/docs/observability.md index dc11a55..8f5f4a5 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -47,7 +47,7 @@ For which signal sets which limit, see [Tuning](tuning.md). - **Scratch space.** Alert on free space on each host's scratch. For a disk-backed scratch, use `node_filesystem_avail_bytes` from the node exporter. For a tmpfs, compare the container's memory usage with the tmpfs `size=`. A full scratch fails every request that needs it. A write that fails inside - libvips gets `unreadable` from the cell, a permanent verdict against the file (see + libvips gets `unreadable` from the cell, a permanent failure against the file (see [ImageMagick](imagemagick.md#how-the-limits-interact-in-a-cell)). [Scratch](scratch.md) covers the layouts. - **Cell errors.** Alert on any `ERROR` event in the cell log, such as `worker.crashed` or @@ -155,7 +155,7 @@ control socket is local to its host, so the process that polls it must run on th `killed_by`, broken down by cause. `killed_by` counts what workers reported, not what the supervisor observed. A worker decides its own -`memory` and `fsize` verdicts, because the supervisor can't tell either from a wait status without +`memory` and `fsize` failures, because the supervisor can't tell either from a wait status without believing a signal that a sibling could have sent. The worker reports the cause when it reports itself idle. As a result: diff --git a/docs/tuning.md b/docs/tuning.md index 14504b8..70a0976 100644 --- a/docs/tuning.md +++ b/docs/tuning.md @@ -48,7 +48,7 @@ test tells you nothing useful about them. - **A limit that's too high costs headroom.** The container's own memory limit still bounds the cell. - **A limit that's too low kills a legitimate file.** The cell answers `killed` with cause `memory` or - `fsize`. Both verdicts are permanent, so the caller is entitled to record them, and the shipped Active + `fsize`. Both failures are permanent, so the caller is entitled to record them, and the shipped Active Storage analyzers do. See [What Active Storage records](codes.md#what-active-storage-records). The two mistakes aren't equal, so use this order: @@ -83,11 +83,11 @@ reports `answer_within`. Which way to fix it depends on the caller: - **A background job** wants a loose timeout, above `answer_within`, so that it receives the cell's - verdict and can act on it. Active Storage's analysis, preview, and variant work all runs in jobs. + failure and can act on it. Active Storage's analysis, preview, and variant work all runs in jobs. - **A synchronous request** wants the opposite: a short `deadline` on the cell and a tight timeout on the client, because a thread held for a minute is a thread that isn't serving traffic. -Both outcomes are transient, so neither choice misclassifies anything. That's the only reason that this +Both failures are transient, so neither choice misclassifies anything. That's the only reason that this is safe to decide for each caller. ## Constraints between the numbers @@ -101,7 +101,7 @@ Nothing checks these constraints for you, except where noted. - `concurrency × the most scratch that one request holds` must be no more than the scratch. `file_size` limits each file, not their sum, and one request can hold a staged input, an output, and a tool's intermediate files at once. Above it, concurrent workers fill the scratch, and requests fail with - `ENOSPC` instead of with a limit verdict. On the default accessory, the + `ENOSPC` instead of with a limit's code. On the default accessory, the scratch is the tmpfs, and its `size=` is the number to fit. See [Scratch](scratch.md#what-changes-in-the-numbers). - `concurrency × (MAGICK_DISK_LIMIT + everything else that one worker writes on scratch)` must be no more than the scratch. Above it, concurrent ImageMagick processes fill the scratch before any of them refuses @@ -152,8 +152,8 @@ service time. bin/load IMAGE [SCENARIO] [SECONDS] [THREADS] ``` -It runs a cell in a container and drives it from a second container. It reports throughput, the verdict -breakdown, and latency split into time queued and time performing. That split separates saturation from +It runs a cell in a container and drives it from a second container. It reports throughput, the count +of each code, and latency split into time queued and time performing. That split separates saturation from slowness: queued time that grows while perform time stays flat means that the cell needs more workers. It can't give you your own numbers. It drives the example operations rather than yours, and it fixes diff --git a/examples/gate b/examples/gate index 0f18691..fab1df8 100755 --- a/examples/gate +++ b/examples/gate @@ -14,7 +14,7 @@ # # 2. bin/conformance and bin/load must supply no `--ulimit stack`. docs/container.md forbids it: a # lowered stack turns an overflow into a SIGSEGV the cell reports as killed/crashed, a transient -# verdict the caller retries against a limit that will fail it again, so a helper that set it would +# failure the caller retries against a limit that will fail it again, so a helper that set it would # measure a shape operators are told not to deploy. # # 3. The battery's overload check must wait for the cell to empty before it fills it. A worker holds its diff --git a/examples/lib/battery.rb b/examples/lib/battery.rb index 311c365..1d40a51 100644 --- a/examples/lib/battery.rb +++ b/examples/lib/battery.rb @@ -146,7 +146,7 @@ def orphan # # The extra request waits until metrics show every place taken rather than for a fixed interval: on a # slow runner a blocker can arrive late, and the extra request then takes its place and succeeds. The - # blockers' own outcomes are not asserted, because the queued ones may run or may wait and either way + # blockers' own answers are not asserted, because the queued ones may run or may wait and either way # the cell is full. They are reported when the cell never fills, so a blocker that failed says why. def overload places = @described.fetch(:concurrency) + @described.fetch(:queue_size) diff --git a/examples/load b/examples/load index 7a9fbbc..eeeec73 100755 --- a/examples/load +++ b/examples/load @@ -3,7 +3,7 @@ # The example operations at volume: the driver half of bin/load, run inside a second container against a # containerized cell. Threads hammer one scenario for a duration, and the report splits what the doc's -# knobs are for — time spent queued against time spent performing, the verdict mix, and throughput. +# knobs are for — time spent queued against time spent performing, the count of each code, and throughput. # # load [SCENARIO] [SECONDS] [THREADS] # @@ -70,9 +70,9 @@ end abort "unknown scenario #{SCENARIO.inspect} (#{CALLS.keys.join(", ")}, mix)" unless SCENARIO == "mix" || CALLS.key?(SCENARIO) -# The verdict, as a caller sees it: `ok`, or the failure's code — with the cause kept for `killed`, +# The code, as a caller sees it: `ok`, or the failure's code — with the cause kept for `killed`, # because `killed: memory` and `killed: deadline` call for different responses to pressure. -def verdict_for(error) +def code_for(error) return "ok" if error.nil? parts = error.message.split(": ") @@ -89,7 +89,7 @@ ActiveSupport::Notifications.subscribe "perform.hot_cell" do |event| recorder.synchronize { samples << sample } end -verdicts = Hash.new(0) +codes = Hash.new(0) started = Process.clock_gettime(Process::CLOCK_MONOTONIC) stop_at = started + SECONDS @@ -104,7 +104,7 @@ THREADS.times.map do raised end - recorder.synchronize { verdicts[verdict_for(error)] += 1 } + recorder.synchronize { codes[code_for(error)] += 1 } end end end.each(&:join) @@ -120,12 +120,12 @@ def percentiles(values) end.join(" ") end -total = verdicts.values.sum +total = codes.values.sum puts puts format("%d requests in %.1fs — %.1f req/s on %d threads", total, elapsed, total / elapsed, THREADS) puts -puts "verdicts:" -verdicts.sort_by { |_, count| -count }.each { |verdict, count| puts format(" %-18s %6d", verdict, count) } +puts "codes:" +codes.sort_by { |_, count| -count }.each { |code, count| puts format(" %-18s %6d", code, count) } puts puts "latency (wall): #{percentiles(samples.map { |sample| sample[:duration_ms] })}" puts "latency (queued): #{percentiles(samples.map { |sample| sample[:queued_ms] })}" diff --git a/examples/operations/greedy.rb b/examples/operations/greedy.rb index 832c550..2c33f45 100644 --- a/examples/operations/greedy.rb +++ b/examples/operations/greedy.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Allocates the payload's megabytes in one buffer — the decompression-bomb shape. Past the cell's memory -# limit the allocation raises NoMemoryError and the verdict is `killed: memory`, where RLIMIT_DATA is +# limit the allocation raises NoMemoryError and the cell answers `killed: memory`, where RLIMIT_DATA is # enforceable. module Examples class Greedy < HotCell::Operation diff --git a/examples/operations/overflow.rb b/examples/operations/overflow.rb index fe7f46f..e1d6a7c 100644 --- a/examples/operations/overflow.rb +++ b/examples/operations/overflow.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Writes the payload's megabytes straight through the caller's output descriptor. Past the cell's -# file-size limit the kernel raises SIGXFSZ and the verdict is `killed: fsize`. +# file-size limit the kernel raises SIGXFSZ and the cell answers `killed: fsize`. module Examples class Overflow < HotCell::Operation operation "example.overflow" diff --git a/hotcell-client-legacy/lib/hot_cell/client/legacy.rb b/hotcell-client-legacy/lib/hot_cell/client/legacy.rb index 64de94b..f0abe87 100644 --- a/hotcell-client-legacy/lib/hot_cell/client/legacy.rb +++ b/hotcell-client-legacy/lib/hot_cell/client/legacy.rb @@ -29,7 +29,7 @@ class Legacy INPUT_MODE = 0o640 OUTPUT_MODE = 0o620 - # A cell's verdict on a request that did not succeed, with the readers of hotcell-core's HotCell::Failure. + # A cell's answer to a request that did not succeed, with the readers of hotcell-core's HotCell::Failure. class Failure attr_reader :code, :cause, :signal, :error_class, :message, :stderr diff --git a/hotcell-client/lib/hot_cell/cell.rb b/hotcell-client/lib/hot_cell/cell.rb index 65b3648..24adf9b 100644 --- a/hotcell-client/lib/hot_cell/cell.rb +++ b/hotcell-client/lib/hot_cell/cell.rb @@ -11,8 +11,8 @@ module HotCell class Cell attr_reader :name, :timeout, :control_timeout, :permanent, :transient, :transport - # `timeout` covers work, so it is sized to clear the cell's `answer_within` and a saturated cell reports - # its own verdict rather than a transport failure. `control_timeout` covers `describe` and `metrics`, + # `timeout` covers work, so it is sized to clear the cell's `answer_within` and a saturated cell answers + # for itself rather than arriving as a transport failure. `control_timeout` covers `describe` and `metrics`, # which the supervisor answers inline with no fork or queue — so it is short on purpose. Sharing one # number would give the call whose job is to say "this cell is down" the patience of a video transcode. # @@ -189,7 +189,7 @@ def control(op) # Being bound tighter than that is defensible and it is a choice, not a mistake. A synchronous # representation request wants it tighter, because a thread held for sixty seconds is a thread not # serving traffic. A background job wants it looser, so it receives `capacity` or `killed` and can act - # on them rather than guessing from a socket error. Both outcomes are transient, so neither is + # on them rather than guessing from a socket error. Both failures are transient, so neither is # misclassified — which is the only reason this is safe. # The cell states this; adding it up here would mean guessing at stages only the supervisor knows about. # A cell too old to report it says nothing, which is the right answer for a number we cannot know. @@ -199,7 +199,7 @@ def warn_about_timeout(described) HotCell.logger.warn "hotcell #{name}: this client waits #{seconds timeout} and the cell says it may " \ "take #{seconds needed} to answer, so a saturated cell will arrive here as a " \ - "transport failure rather than as its own verdict. Deliberate on a synchronous " \ + "transport failure rather than as the cell's own answer. Deliberate on a synchronous " \ "path; a mistake for a background job." end diff --git a/hotcell-client/lib/hot_cell/client.rb b/hotcell-client/lib/hot_cell/client.rb index 0130d84..166d721 100644 --- a/hotcell-client/lib/hot_cell/client.rb +++ b/hotcell-client/lib/hot_cell/client.rb @@ -168,7 +168,7 @@ def verify_output(response, outputs) # rate you cannot size a worker pool. It is `ok` on success, so no subscriber reads success from a # missing code. # - # The cause, signal and verdict go with it, because the code alone cannot classify a kill: `killed` + # The cause, signal and `permanent` go with it, because the code alone cannot classify a kill: `killed` # is permanent for fsize and memory and transient for deadline and crashed. A subscriber with only # the code filed every fsize kill as transient. `permanent` is the Failure's own answer, so no # subscriber re-derives it. diff --git a/hotcell-client/test/classification_test.rb b/hotcell-client/test/classification_test.rb index a8512d8..640e3a6 100644 --- a/hotcell-client/test/classification_test.rb +++ b/hotcell-client/test/classification_test.rb @@ -3,14 +3,14 @@ require "test_helper" require "timeout" -# Every failure a client can produce, classified. The transport is a seam, so this drives each verdict +# Every failure a client can produce, classified. The transport is a seam, so this drives each failure # straight through it rather than arranging a cell to produce it. # # Anything a client can raise that the consuming application does not classify becomes a user-facing 500. # That is not hypothetical: applications carry rails_ext patches that exist because # `system(exception: true)` raised a bare RuntimeError outside their rescue lists. class ClassificationTest < HotCellClientTest - # Only the verdicts something actually knew: an operation naming what it could not decode, a caller + # Only the failures something actually knew: an operation naming what it could not decode, a caller # breaking the protocol, and the two limits this request provably hit by itself. PERMANENT = [ { code: "unreadable" }, @@ -154,8 +154,8 @@ def test_the_event_carries_the_code_even_though_the_call_raised # `code` alone cannot classify a kill: `killed` is permanent for fsize and memory and transient for # deadline and crashed. A subscriber that only had the code filed every fsize kill as transient. So the - # event carries what the Failure knows — the cause, the signal, and the verdict itself. - def test_the_event_carries_the_cause_and_the_verdict_so_a_subscriber_can_classify_a_kill + # event carries what the Failure knows — the cause, the signal, and `permanent` itself. + def test_the_event_carries_the_cause_and_permanent_so_a_subscriber_can_classify_a_kill register_with failed(code: "killed", cause: "fsize", signal: "XFSZ") event = events_for do @@ -181,7 +181,7 @@ def test_the_event_carries_the_stderr_so_a_subscriber_can_log_the_diagnosis_of_a assert_equal "libgomp: Thread creation failed\n", event[:stderr] end - def test_the_event_carries_a_transient_verdict_for_a_deadline_kill + def test_the_event_marks_a_deadline_kill_transient register_with failed(code: "killed", cause: "deadline") event = events_for do @@ -251,9 +251,9 @@ class Anything < HotCell::Client # The cell is the untrusted side, so its answer is untrusted bytes and every way of failing to read one # has to land inside the taxonomy. JSON.parse raises EncodingError rather than JSON::ParserError for a # key holding bytes that are not valid UTF-8, and Transport::Socket rescues neither by name — so a - # hostile cell could raise an exception in the application that no code, no verdict and no + # hostile cell could raise an exception in the application that no code, no failure and no # `perform.hot_cell` event described, past whatever the caller had arranged to rescue. - def test_an_answer_that_cannot_be_read_is_a_verdict_rather_than_an_escaping_exception + def test_an_answer_that_cannot_be_read_is_a_failure_rather_than_an_escaping_exception hostile = %({"v":1,"ok":true,"result":{"\xff\xfe":1},"timing":{}}\n).b with_cell_answering(hostile) do @@ -266,9 +266,9 @@ def test_an_answer_that_cannot_be_read_is_a_verdict_rather_than_an_escaping_exce end # A full cell writes its answer and closes the connection without reading the request. The client's send - # then fails, but the answer is already on its socket. The answer is the verdict. A request larger than the + # then fails, but the answer is already on its socket. The answer is what counts. A request larger than the # socket buffer makes the send fail every time. - def test_an_answer_that_arrives_before_the_request_is_sent_is_still_the_verdict + def test_an_answer_that_arrives_before_the_request_is_sent_is_still_the_answer response = call_a_full_cell assert_equal "capacity", response.failure.code, response.failure.to_s @@ -276,7 +276,7 @@ def test_an_answer_that_arrives_before_the_request_is_sent_is_still_the_verdict # macOS reports the same early close as ENOTCONN rather than EPIPE when the close lands while its kernel is # copying the request in. That window is narrow, so the test above passes there almost every time. - def test_an_answer_that_arrives_before_a_send_refused_as_not_connected_is_still_the_verdict + def test_an_answer_that_arrives_before_a_send_refused_as_not_connected_is_still_the_answer failing_sends_with Errno::ENOTCONN do response = call_a_full_cell @@ -320,7 +320,7 @@ def failing_sends_with(error) # # It reads the request before answering, the way a real supervisor does. Answering without reading # raced the client: a close that landed before the client's `sendmsg` gave it EPIPE on the write leg, - # so it produced a verdict for a broken pipe and never reached the read this test is about. + # so it produced a failure for a broken pipe and never reached the read this test is about. def with_cell_answering(line, name: "test") Dir.mktmpdir "hotcell-hostile" do |root| directory = File.join(root, name) diff --git a/hotcell-core/lib/hot_cell/codes.rb b/hotcell-core/lib/hot_cell/codes.rb index 106c0c7..520df4e 100644 --- a/hotcell-core/lib/hot_cell/codes.rb +++ b/hotcell-core/lib/hot_cell/codes.rb @@ -82,10 +82,10 @@ module Codes # # So the supervisor never infers either of them from a signal. They are decided in the worker, which is # the process that holds the request: `memory` when it catches NoMemoryError itself, and `fsize` when a - # write of its own returns EFBIG. See Worker#disarm_file_size_signal for why the file-size verdict has + # write of its own returns EFBIG. See Worker#disarm_file_size_signal for why the `fsize` failure has # to be earned that way rather than read off a wait status. # - # That is a narrower guarantee than "a permanent verdict cannot be forged", and the difference matters. + # That is a narrower guarantee than "a permanent failure cannot be forged", and the difference matters. # It removes the supervisor as an instrument: a sibling's signal no longer travels through a wait status # into someone else's blob. It does nothing about a cell that has been compromised outright and answers # a connection itself — `from_wire` believes a `permanent` boolean off the wire, and a worker that stole diff --git a/hotcell-core/lib/hot_cell/descriptors.rb b/hotcell-core/lib/hot_cell/descriptors.rb index 07008cb..18afec4 100644 --- a/hotcell-core/lib/hot_cell/descriptors.rb +++ b/hotcell-core/lib/hot_cell/descriptors.rb @@ -117,7 +117,7 @@ class Input < Descriptor # Copies the bytes onto the worker's own scratch on the first call and returns the filename. This is the # fallback, for an operation that genuinely needs a real file. Prefer `fd_path`, which reads the # descriptor in place: staging is a write, so RLIMIT_FSIZE bounds it, and an input larger than the - # operation's file_size dies here as a permanent `fsize` verdict — a ceiling Rails does not have. The + # operation's file_size dies here as a permanent `fsize` failure — a ceiling Rails does not have. The # Active Storage operations all read `fd_path` for exactly that reason; nothing should reach for `path` # without a specific need for a distinct on-disk copy. # diff --git a/hotcell-core/lib/hot_cell/errors.rb b/hotcell-core/lib/hot_cell/errors.rb index fe38cb0..b26011a 100644 --- a/hotcell-core/lib/hot_cell/errors.rb +++ b/hotcell-core/lib/hot_cell/errors.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true module HotCell - # What this gem raises when a call is wrong, as distinct from a cell's verdict on a conversion. + # What this gem raises when a call is wrong, as distinct from a cell's failure. # # These must be raised outside the client's transport rescue. An application injects its own - # exception classes for a cell's verdicts, and if one of those descends from IOError then a bad call + # exception classes for a cell's failures, and if one of those descends from IOError then a bad call # swallowed by the transport rescue comes back as a socket failure and gets retried forever. class Error < StandardError; end @@ -24,7 +24,7 @@ class MessageError < Error; end # cannot hold its limits and a client that cannot classify a failure are both worse discovered in traffic. class ConfigurationError < Error; end - # A peer that stopped mid-message. Distinct from a cell answering `timeout`, which is a verdict: this is + # A peer that stopped mid-message. Distinct from a cell answering `timeout`, which is a failure: this is # the caller's own deadline passing with the response incomplete. class ReadTimeout < Error; end diff --git a/hotcell-core/lib/hot_cell/failure.rb b/hotcell-core/lib/hot_cell/failure.rb index 7274214..6a2f8f5 100644 --- a/hotcell-core/lib/hot_cell/failure.rb +++ b/hotcell-core/lib/hot_cell/failure.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module HotCell - # A cell's verdict on a request that did not succeed. + # A cell's answer to a request that did not succeed. # # The message is untrusted and it outlives the request. It comes out of a worker that has just parsed # a hostile file, and Vips::Error#message routinely contains the input filename. Applications store @@ -71,7 +71,7 @@ def for(code, detail, cause: nil) # A code this client has never heard of is not permanent. An old client will meet a code added # later, and the harm of the two mistakes is not symmetrical: retrying something permanent costs - # some work, while writing down a verdict that was temporary is irreversible. + # some work, while writing down a failure that was temporary is irreversible. # A `permanent` that is present but not a boolean is derived rather than believed. Truthiness would make # any non-nil value permanent, and permanent is the answer that cannot be taken back — so a garbled # field must not be able to say it. diff --git a/hotcell-core/lib/hot_cell/payload.rb b/hotcell-core/lib/hot_cell/payload.rb index 46c265e..6d0d76c 100644 --- a/hotcell-core/lib/hot_cell/payload.rb +++ b/hotcell-core/lib/hot_cell/payload.rb @@ -33,7 +33,7 @@ def generate(object, name) # cell. # # The body is one JSON.parse call, so the rescue cannot hide a bug of our own. NoMemoryError is not - # a StandardError and stays uncaught: a document that large is the worker's memory verdict, not a + # a StandardError and stays uncaught: a document that large is the worker's `memory` failure, not a # bad line. def parse(json) JSON.parse json, symbolize_names: true, max_nesting: MAX_NESTING, allow_nan: false diff --git a/hotcell-core/test/codes_test.rb b/hotcell-core/test/codes_test.rb index 554d32c..58644a3 100644 --- a/hotcell-core/test/codes_test.rb +++ b/hotcell-core/test/codes_test.rb @@ -20,7 +20,7 @@ def test_the_codes_that_mean_try_again # `failed` is whatever an unclassified exception became. Errno::ENOSPC is a StandardError, so a full tmpfs # or a full disk under the caller's output arrives here — and permanent meant those were written down # against a customer's file forever, for a condition that would have succeeded on retry. - def test_an_unclassified_exception_is_not_a_verdict_on_the_input + def test_an_unclassified_exception_is_not_permanent refute HotCell::Codes.permanent?("failed") end @@ -28,7 +28,7 @@ def test_an_unclassified_exception_is_not_a_verdict_on_the_input # anybody reboots the cell gets this at one hundred percent for as long as that takes. Recording it as # permanent condemns every blob uploaded during the window, and a caller's typo shows up in the rate and in # the refusal, which names the operation. - def test_an_operation_a_cell_does_not_carry_yet_is_a_deploy_window_rather_than_a_verdict + def test_an_operation_a_cell_does_not_carry_yet_is_a_deploy_window_rather_than_permanent refute HotCell::Codes.permanent?("unsupported") end @@ -40,9 +40,9 @@ def test_killed_answers_from_the_limit_the_worker_hit # A signal says how a process died, never why. The supervisor names its own deadline kill; anything else # arrived from a cgroup OOM chosen on aggregate pressure, or from a sibling worker sharing this uid. - # Reading that as a verdict condemns an input for something it did not do — so it is `crashed`, the same + # Reading that as permanent condemns an input for something it did not do — so it is `crashed`, the same # answer as a worker that exited without saying anything, because it is the same amount of knowledge. - def test_a_worker_dying_for_an_unattributable_reason_is_not_a_verdict_on_the_input + def test_a_worker_dying_for_an_unattributable_reason_is_not_permanent refute HotCell::Codes.permanent?("killed", cause: "crashed") end @@ -58,7 +58,7 @@ def test_a_writer_asking_for_an_unknown_code_is_a_typo_rather_than_forward_compa end # The whole point of the table's default. A cell mints these, and a kill reason added to the supervisor - # without a row here is a forgotten row rather than a verdict — so it must not come out permanent, which is + # without a row here is a forgotten row rather than a decision — so it must not come out permanent, which is # the answer that cannot be taken back. def test_a_limit_the_table_has_never_heard_of_is_not_permanent refute HotCell::Codes.permanent?("killed", cause: "oom") diff --git a/hotcell-core/test/descriptors_test.rb b/hotcell-core/test/descriptors_test.rb index 12831fa..f9f69f9 100644 --- a/hotcell-core/test/descriptors_test.rb +++ b/hotcell-core/test/descriptors_test.rb @@ -124,7 +124,7 @@ def test_posting_a_staged_output_sends_what_the_operation_wrote_and_reports_the_ end end - # Zero bytes is a verdict the client already has to handle, because a full tmpfs arrives the same way. + # Zero bytes is a failure the client already has to handle, because a full tmpfs arrives the same way. def test_posting_a_staged_output_the_operation_never_wrote_reports_zero with_file do |destination| Dir.mktmpdir do |scratch| diff --git a/hotcell-server/lib/hot_cell/configuration.rb b/hotcell-server/lib/hot_cell/configuration.rb index 323ab3a..1e53670 100644 --- a/hotcell-server/lib/hot_cell/configuration.rb +++ b/hotcell-server/lib/hot_cell/configuration.rb @@ -83,7 +83,7 @@ def retire?(served) # Derived here rather than reassembled in the client: a client adding up `queue_wait + deadline` plus its # own guess at the kill-to-answer step cannot follow a change to any of them, and the error points the # unsafe way — the client believes its timeout is generous and takes a transport failure instead of the - # cell's verdict. A stage added later that costs a caller time belongs in this sum. + # cell's answer. A stage added later that costs a caller time belongs in this sum. def answer_within queue_wait + limits.deadline + KILL_GRACE end diff --git a/hotcell-server/lib/hot_cell/operation.rb b/hotcell-server/lib/hot_cell/operation.rb index f1c22b5..6ccf720 100644 --- a/hotcell-server/lib/hot_cell/operation.rb +++ b/hotcell-server/lib/hot_cell/operation.rb @@ -147,7 +147,7 @@ def ok? # streams in full and hands them over at exit, so slicing afterwards bounded the Strings this method # returns and nothing else: an input that makes a tool print gigabytes of diagnostics had already # cost gigabytes of this worker's address space, and took RLIMIT_DATA with it — arriving as a `memory` - # verdict, which is permanent, for a document whose only crime was being noisy. + # failure, which is permanent, for a document whose only crime was being noisy. # `pass` hands the tool a set of the worker's own descriptors — an input to read, an output to write — # at their existing fd numbers, so the tool reaches them at `Descriptor#fd_path` — `/dev/fd/N`, or the # file's own path on macOS, where opening `/dev/fd/N` would share the worker's offset — and no diff --git a/hotcell-server/lib/hot_cell/server/errors.rb b/hotcell-server/lib/hot_cell/server/errors.rb index a79813d..c036497 100644 --- a/hotcell-server/lib/hot_cell/server/errors.rb +++ b/hotcell-server/lib/hot_cell/server/errors.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module HotCell - # What an operation raises to choose its own verdict. An operation may also declare the library + # What an operation raises to choose its own failure. An operation may also declare the library # exceptions that mean the same thing, with `unreadable Vips::Error`. class OperationError < StandardError; end diff --git a/hotcell-server/lib/hot_cell/supervisor.rb b/hotcell-server/lib/hot_cell/supervisor.rb index 312283b..72f1975 100644 --- a/hotcell-server/lib/hot_cell/supervisor.rb +++ b/hotcell-server/lib/hot_cell/supervisor.rb @@ -427,7 +427,7 @@ def pump end # A worker can die between the fork and this write. Answering rather than raising is what keeps one dead - # worker from taking the whole cell down with it, and the caller gets a transient verdict either way. + # worker from taking the whole cell down with it, and the caller gets a transient failure either way. def dispatch(child, connection, queued_ms) child.dispatched connection, configuration.limits.deadline, at: Clock.now @@ -784,7 +784,7 @@ def unreadable_report(child, message) end def finish(child, code, cause = nil) - counters.record outcome_code(code) + counters.record reported_code(code) counters.record_kill cause if reported_cause?(code, cause) child.finished discard child @@ -792,7 +792,7 @@ def finish(child, code, cause = nil) retire child if configuration.retire?(child.served) end - # The outcome code rides an untrusted worker report and becomes both a counter key and a Symbol, so it + # The code rides an untrusted worker report and becomes both a counter key and a Symbol, so it # is bounded to a code this cell mints before either happens. Without this a report of `code: []` raised # NoMethodError on `to_sym` past `apply_report`'s rescue and took the cell down, and a stream of unique # strings grew the counters without bound. An unknown code is recorded, so a misreporting worker is @@ -801,7 +801,7 @@ def finish(child, code, cause = nil) # It is checked against the known causes rather than passed through, because `record_kill` interns it # as a symbol and an unchecked one is an unbounded symbol table keyed by whatever a tool decides. def reported_cause?(code, cause) - outcome_code(code) == Codes::KILLED && cause.is_a?(String) && + reported_code(code) == Codes::KILLED && cause.is_a?(String) && Codes::PERMANENT_BY_CAUSE.key?(cause) end @@ -810,7 +810,7 @@ def reported_op(reported) reported if reported.is_a?(String) && Registry.lookup(reported) end - def outcome_code(reported) + def reported_code(reported) return reported if reported.is_a?(String) && (reported == "ok" || Codes.known?(reported)) "unknown" @@ -853,7 +853,7 @@ def enforce_retirements now = Clock.now @children.each_value.select { |child| child.lingering?(now, Configuration::KILL_GRACE) }.each do |child| - # The latch rather than a verdict: a lingering child is never busy, so no caller hears this cause. + # The latch rather than an answer: a lingering child is never busy, so no caller hears this cause. child.killed_for = Codes::CRASHED kill_group child @@ -968,7 +968,7 @@ def reap end # A killed worker cannot report its own death, because the deadline KILL is enforced by a signal. So - # the supervisor holds its copy of every dispatched connection and writes the verdict itself. Without + # the supervisor holds its copy of every dispatched connection and writes the answer itself. Without # this the cold side sees a bare end of stream and cannot tell a limit # breach from a crash. # A worker still holding a connection at reap time never answered: it reports itself idle after writing, @@ -978,10 +978,10 @@ def reap # supervisor sent, and it is the one thing here that knows why a worker died. Everything else is a # wait status, which says how — and workers share a uid, so any signal in one may have come from a # sibling rather than from the kernel. Reading XFSZ, SEGV, ABRT or TRAP as this request's file size or - # memory let one compromised worker write a permanent verdict against another request's unrelated + # memory let one compromised worker write a permanent failure against another request's unrelated # input, which Active Storage then kept. `Codes` already stated the rule this broke. # - # The verdicts themselves are not gone, they moved to where they can be earned: the worker answers + # The failures themselves are not gone, they moved to where they can be earned: the worker answers # `memory` when it catches NoMemoryError and `fsize` when a write of its own returns EFBIG, on the # connection it is holding. See Worker#disarm_file_size_signal. # @@ -1001,7 +1001,7 @@ def answer_for(child, status) signal: signal_name(status), duration_ms: Clock.ms_since(child.dispatched_at), **captured - # The capture rides the verdict as well as the log line, so an application logs + # The capture rides the failure as well as the log line, so an application logs # `killed: crashed (libgomp: ...)` rather than a bare `crashed`. Additive on the wire: `from_wire` # reads named keys, so an old client ignores the field and a new one against an old cell meets nil. answer child.connection, diff --git a/hotcell-server/lib/hot_cell/test_operations.rb b/hotcell-server/lib/hot_cell/test_operations.rb index f88f0ed..e64a766 100644 --- a/hotcell-server/lib/hot_cell/test_operations.rb +++ b/hotcell-server/lib/hot_cell/test_operations.rb @@ -187,7 +187,7 @@ def perform(_inputs, _outputs) # A tool spawn that fails because the host is out of memory. fork raises Errno::ENOMEM under host # pressure, before the tool has read a byte of the input, so this is the cell having a bad moment - # rather than a decompression bomb — and it must not be recorded as a permanent memory verdict. + # rather than a decompression bomb — and it must not be recorded as a permanent `memory` failure. class StarvedSpawn < HotCell::Operation operation "test.starved_spawn" @@ -200,7 +200,7 @@ class SignalsSibling < HotCell::Operation operation "test.signals_sibling" # Workers share a uid and a pid namespace, so one finds another by looking for a process the - # supervisor also fathered. This is the reproducer for the forged verdict: nothing here touches the + # supervisor also fathered. This is the reproducer for the forged failure: nothing here touches the # victim's input, and the victim is holding an unrelated one. def perform(_inputs, _outputs, signal:) sibling = siblings.first diff --git a/hotcell-server/lib/hot_cell/timing.rb b/hotcell-server/lib/hot_cell/timing.rb index 3d37bcd..e7767ea 100644 --- a/hotcell-server/lib/hot_cell/timing.rb +++ b/hotcell-server/lib/hot_cell/timing.rb @@ -11,7 +11,7 @@ module HotCell # here, when `performing` is called. # # Phases accumulate as they complete, so a refusal reports the ones that finished before the failure. An - # `unreadable` verdict that arrives with no `operation_ms` says exactly where it got to. + # `unreadable` failure that arrives with no `operation_ms` says exactly where it got to. class Timing attr_reader :queued_ms, :started diff --git a/hotcell-server/lib/hot_cell/worker.rb b/hotcell-server/lib/hot_cell/worker.rb index 732e48e..2cffe49 100644 --- a/hotcell-server/lib/hot_cell/worker.rb +++ b/hotcell-server/lib/hot_cell/worker.rb @@ -29,7 +29,7 @@ def initialize(slot:, configuration:, control:, log:) # # A non-zero status for anything unexpected, because the supervisor holds the connection and is the only # thing that can answer for a worker that died mid-request. Exiting zero here would leave a caller reading - # a closed socket with no verdict at all. + # a closed socket with no answer at all. # # **The one deliberate `rescue Exception` in this repository.** Not for the exit status — Ruby's own handler # would also exit non-zero — but for Failure.sanitize. Left to Ruby, a NoMemoryError or a SystemStackError @@ -67,8 +67,8 @@ def run # # Catching it makes the kernel fail the offending write with EFBIG rather than killing the process, # and that error return is what a signal is not: it is raised by a write this process made, and no - # signal any sibling sends produces one. So the verdict below keys on Errno::EFBIG and this handler - # does nothing at all. A handler that set so much as a flag the verdict consulted would hand the + # signal any sibling sends produces one. So the `fsize` failure below keys on Errno::EFBIG and this handler + # does nothing at all. A handler that set so much as a flag that failure consulted would hand the # forgery straight back. # # EFBIG is evidence of this request's own write and not proof of which limit stopped it. A filesystem @@ -137,7 +137,7 @@ def serve(connection, queued_ms) # transient. Adding it back would condemn a blob for the cell's own bad moment. rescue NoMemoryError, MemoryExhausted => error response = refuse(Codes::KILLED, error, timing, cause: Codes::MEMORY) - # The one place a file-size verdict can be earned. EFBIG comes back from a write this worker made + # The one place an `fsize` failure can be earned. EFBIG comes back from a write this worker made # past its own RLIMIT_FSIZE, so unlike the signal it cannot arrive from anywhere else. rescue Errno::EFBIG => error response = refuse(Codes::KILLED, error, timing, cause: Codes::FSIZE) @@ -229,7 +229,7 @@ def perform(operation, inputs, outputs, payload, timing) # reads as success. This is the side that knows which one is empty, so it is the side that says so. # # Transient, for the reason the client's own check is: the commonest way to write nothing is a full - # tmpfs, and a full filesystem must never be recorded as a verdict on the document. + # tmpfs, and a full filesystem must never be recorded as a permanent failure of the document. def unwritten(outputs, written, timing) empty = written.each_index.select { |index| written[index].zero? } @@ -295,7 +295,7 @@ def fits?(message) # that off a wait status; it reads the worker's own report now, and only when there is a cause to send, # so an ordinary idle report is the two keys it always was. # - # **This is a metric and not a verdict.** The verdict went to the caller on the work connection before + # **This is a metric and not an answer.** The answer went to the caller on the work connection before # this line runs. A compromised worker can report a cause its request never had, or withhold one it # did, so `killed_by` is what workers said rather than what happened — which is why the supervisor # checks the value against the known causes before interning it, and why nothing downstream may treat diff --git a/hotcell-server/test/cell_test.rb b/hotcell-server/test/cell_test.rb index 860a898..71d5a4a 100644 --- a/hotcell-server/test/cell_test.rb +++ b/hotcell-server/test/cell_test.rb @@ -92,7 +92,7 @@ def test_an_operation_that_reads_the_descriptor_rather_than_a_copy # application that ships a client for a new operation before anybody reboots the cell gets this at one # hundred percent until they do — and recording that as permanent condemns every blob uploaded during the # window. A caller's typo shows up in the `unsupported` rate and in the refusal, which names the operation. - def test_an_unknown_operation_is_a_deploy_window_rather_than_a_verdict_on_the_input + def test_an_unknown_operation_is_a_deploy_window_rather_than_a_permanent_failure TestCell.boot do |cell| failure = assert_failed "unsupported", cell.call("test.nonexistent") @@ -125,7 +125,7 @@ def test_an_operation_that_raises end # The payload arrives as keyword arguments, so a key the operation did not declare raises inside - # perform. Transient, like every unclassified raise: a caller bug must never become a verdict. + # perform. Transient, like every unclassified raise: a caller bug must never become a permanent failure. def test_a_payload_key_the_operation_does_not_declare_answers_failed TestCell.boot do |cell| failure = assert_failed "failed", cell.call("test.blocking", payload: { hours: 1 }) @@ -153,7 +153,7 @@ def test_a_library_exception_an_operation_declared_as_unreadable end end - # A refusal reports only the phases that finished before it, so a verdict says where the request got to + # A refusal reports only the phases that finished before it, so a failure says where the request got to # rather than only that it failed. The operation never finished here, and posting never started. def test_a_refusal_reports_only_the_phases_that_completed TestCell.boot do |cell| @@ -168,7 +168,7 @@ def test_a_refusal_reports_only_the_phases_that_completed end end - # Memory belongs with the resource verdicts rather than in `failed`, because it is the + # Memory belongs with the resource kills rather than in `failed`, because it is the # decompression-bomb case and a caller must be able to act on it without parsing a message. def test_running_out_of_memory_is_killed_rather_than_failed TestCell.boot do |cell| @@ -181,7 +181,7 @@ def test_running_out_of_memory_is_killed_rather_than_failed # A spawn that fails with ENOMEM is the host out of memory, not the input being a bomb, so it must be # transient. Recording it as permanent memory would condemn a blob for the cell's own bad moment, the way # ENOSPC or EMFILE would if they were not already transient. - def test_a_spawn_starved_of_memory_is_transient_rather_than_a_memory_verdict + def test_a_spawn_starved_of_memory_is_transient_rather_than_a_memory_kill TestCell.boot do |cell| failure = assert_failed "failed", cell.call("test.starved_spawn") @@ -222,7 +222,7 @@ def test_a_result_json_cannot_carry # The cell says so rather than leaving it to the caller to notice. It used to answer `ok` here and let the # client compare the output's size, which cannot see which of several outputs was the empty one. A full - # tmpfs arrives this way, so it is transient rather than a verdict on the document. + # tmpfs arrives this way, so it is transient rather than a permanent failure of the document. def test_an_operation_that_writes_nothing_is_refused_rather_than_reported_ok TestCell.boot do |cell| with_files do |source, destination| @@ -302,7 +302,7 @@ def test_a_result_carrying_bytes_json_cannot_encode_is_reported_rather_than_kill # A worker that dies mid-request without a signal is the cell's fault rather than the input's, and a # misconfigured cell does it on every request. Recording that against a blob would condemn everything - # uploaded during a broken deploy, so it is the one `killed` verdict that is not permanent alongside the + # uploaded during a broken deploy, so it is the one `killed` cause that is not permanent alongside the # deadline. def test_a_worker_that_dies_without_answering_is_reported_and_is_not_permanent TestCell.boot do |cell| @@ -324,7 +324,7 @@ def test_the_cell_serves_normally_after_a_worker_vanishes # A worker that exits with a dispatch queued unread on its control socket resets it, and the # supervisor's read raised Errno::ECONNRESET through the run loop — one dead worker ended the cell # and every in-flight request with it. A reset says what end of stream says: the worker is gone. The - # reap answers the request it was holding, and the verdict is transient. + # reap answers the request it was holding, and the failure is transient. def test_a_worker_that_exits_with_a_dispatch_queued_does_not_take_the_cell_down with_file do |pid_path| TestCell.boot(deadline: 30, concurrency: 1, max_requests_per_worker: 3) do |cell| @@ -434,7 +434,7 @@ def test_a_killed_workers_files_are_taken_out_of_the_way_rather_than_deleted_in_ # workers share a uid, so one can signal another — which the supervisor then read as the victim's own # memory exhaustion and wrote against the victim's unrelated input, permanently. Nothing the attacker does # here touches that input. - def test_a_sibling_signal_is_not_a_verdict_on_the_victims_input + def test_a_sibling_signal_is_not_a_permanent_failure_of_the_victims_input TestCell.boot(concurrency: 2, queue_size: 4) do |cell| victim = Thread.new { cell.call("test.blocking", payload: { seconds: 3 }, timeout: 30) } sleep 0.3 @@ -449,11 +449,11 @@ def test_a_sibling_signal_is_not_a_verdict_on_the_victims_input end # The same forgery with the one signal that has a real meaning. `XFSZ` is what the kernel raises when a - # write passes RLIMIT_FSIZE, and that verdict is permanent — but a sibling sends it just as easily and a + # write passes RLIMIT_FSIZE, and that failure is permanent — but a sibling sends it just as easily and a # wait status cannot tell the two apart. Catching it in the worker answers both halves at once: the # kernel's XFSZ comes back as EFBIG from the write that caused it, and a sibling's does not reach the # victim's request at all, because nothing is listening for it. - def test_a_sibling_can_not_forge_a_file_size_verdict + def test_a_sibling_can_not_forge_an_fsize_failure TestCell.boot(concurrency: 2, queue_size: 4) do |cell| victim = Thread.new { cell.call("test.blocking", payload: { seconds: 3 }, timeout: 30) } sleep 0.3 @@ -465,7 +465,7 @@ def test_a_sibling_can_not_forge_a_file_size_verdict end # The API takes several outputs and success was inferred from their total size, so writing the first and - # skipping the second was a positive total and read as `ok`. Transient rather than a verdict: the + # skipping the second was a positive total and read as `ok`. Transient rather than permanent: the # commonest way to write nothing is a full tmpfs. def test_an_output_that_received_nothing_is_not_success TestCell.boot do |cell| @@ -565,7 +565,7 @@ def test_a_tools_temp_files_are_removed_with_the_home_of_a_request_that_answers def test_a_tools_temp_files_are_removed_with_the_home_of_a_request_that_is_killed with_cell_spilling_into_its_root(deadline: 1) do |cell, untouched| assert_failed "killed", cell.call("test.spills", payload: { seconds: 30 }, timeout: 30) - # The verdict is written before the supervisor discards the home; `worker.reaped` follows the discard. + # The answer is written before the supervisor discards the home; `worker.reaped` follows the discard. refute_empty wait_for_event(cell, "worker.reaped"), "the killed worker was never reaped" assert_empty Dir.children(cell.socket_root) - untouched - [ "workspace" ], diff --git a/hotcell-server/test/control_test.rb b/hotcell-server/test/control_test.rb index 22a722e..55a482e 100644 --- a/hotcell-server/test/control_test.rb +++ b/hotcell-server/test/control_test.rb @@ -26,7 +26,7 @@ def test_describe_reports_which_build_of_the_cell_is_running # response and the supervisor increments the counter when it later reads that worker's idle report — two # processes — so a caller can be holding its answer before the count exists. Anything reading these for # alarms should expect the same skew. - def test_metrics_count_the_outcomes + def test_metrics_count_each_code TestCell.boot do |cell| 2.times { assert_ok cell.call("test.echo") } assert_failed "failed", cell.call("test.broken") @@ -53,8 +53,8 @@ def test_metrics_separate_a_decompression_bomb_from_a_slow_afternoon cause: "fsize" end - # Polled: the worker answers this verdict itself now, so the caller has its response before the - # supervisor has read the idle report the count comes from. Every non-killed outcome was already + # Polled: the worker answers this failure itself now, so the caller has its response before the + # supervisor has read the idle report the count comes from. Every code but `killed` was already # counted on that report; this moves `fsize` into the same place rather than into a new one. result = wait_for_metrics(cell) { |metrics| metrics[:killed_by][:fsize] == 1 } @@ -64,7 +64,7 @@ def test_metrics_separate_a_decompression_bomb_from_a_slow_afternoon end end - # The one outcome that appears in no response, by definition: nobody is left to receive it. A cell quietly + # The one count that appears in no response, by definition: nobody is left to receive it. A cell quietly # doing work for callers who have gone away can only be seen from here. def test_metrics_count_a_caller_that_gave_up_before_the_cell_answered TestCell.boot(deadline: 1) do |cell| diff --git a/hotcell-server/test/environment_test.rb b/hotcell-server/test/environment_test.rb index fc8c3a2..8d59a57 100644 --- a/hotcell-server/test/environment_test.rb +++ b/hotcell-server/test/environment_test.rb @@ -93,7 +93,7 @@ def test_a_tool_gets_no_openmp_bound_the_cell_does_not_have # capture3 accumulated both streams in full and handed them over at exit, so slicing afterwards bounded # only the Strings this method returned. An input that made a tool print 40MB of diagnostics had # already cost this worker 40MB of address space — which RLIMIT_DATA charges, arriving as a `memory` - # verdict for a document whose only crime was being noisy. + # failure for a document whose only crime was being noisy. def test_a_noisy_tool_costs_a_bounded_amount_of_the_workers_memory TestCell.boot do |cell| %w[ out err ].each do |stream| diff --git a/hotcell-server/test/resource_limits_test.rb b/hotcell-server/test/resource_limits_test.rb index f40a0f6..88f4270 100644 --- a/hotcell-server/test/resource_limits_test.rb +++ b/hotcell-server/test/resource_limits_test.rb @@ -102,7 +102,7 @@ def test_core_dumps_are_off # RLIMIT_FSIZE is enforced by a signal, and the supervisor used to read that signal off a wait status — # which a sibling can forge, because workers share a uid. The worker catches SIGXFSZ instead, which makes # the kernel fail the offending write with EFBIG rather than killing the process, and answers for itself - # over its own control socket. So the verdict survives and carries no signal: the write that caused it is + # over its own control socket. So the failure survives and carries no signal: the write that caused it is # the evidence, not the death. def test_writing_past_file_size_is_killed_rather_than_truncated boot do |cell| @@ -112,7 +112,7 @@ def test_writing_past_file_size_is_killed_rather_than_truncated failure = assert_failed "killed", response, cause: "fsize" assert_predicate failure, :permanent? - assert_nil failure.signal, "the verdict was inferred from a signal rather than earned" + assert_nil failure.signal, "the failure was inferred from a signal rather than earned" assert_equal "Errno::EFBIG", failure.error_class end end @@ -134,7 +134,7 @@ def test_file_size_bounds_the_input_copy_too # An allocation past RLIMIT_DATA is a catchable NoMemoryError rather than a signal, which makes it # tempting to report as an ordinary failure. It is the decompression-bomb case, so it belongs with the - # other resource verdicts where a caller can act on it without parsing a message. + # other resource kills where a caller can act on it without parsing a message. def test_allocating_past_the_memory_limit_is_killed_rather_than_failed skip MEMORY_UNENFORCED unless memory_clamp_enforced? diff --git a/hotcell-server/test/scheduling_test.rb b/hotcell-server/test/scheduling_test.rb index 141f81c..e0b061a 100644 --- a/hotcell-server/test/scheduling_test.rb +++ b/hotcell-server/test/scheduling_test.rb @@ -61,7 +61,7 @@ def test_a_deadline_kills_what_the_worker_started_too # macOS refuses a signal to a process group whose only members are zombies, with EPERM rather than ESRCH. # A killed worker's tool is one such zombie until launchd reaps it, so the reap's sweep raised, unwound the - # run loop and ended the cell, and the caller read end of stream instead of the verdict. + # run loop and ended the cell, and the caller read end of stream instead of the answer. def test_a_deadline_kill_is_answered_when_the_reap_sweep_is_refused refuse_group_signals(after: 1) do TestCell.boot(deadline: 0.3, concurrency: 1) do |cell| @@ -250,7 +250,7 @@ def test_a_refusal_is_transient_so_a_restarting_cell_never_condemns_a_document end # Without queue_wait a queued connection waits until the client's own timeout fires, so the client - # reports a transport error and the cell's capacity verdict is never delivered — making the code the + # reports a transport error and the cell's `capacity` failure is never delivered — making the code the # queue exists to surface unreachable exactly when the cell is saturated. def test_a_connection_that_waits_too_long_is_told_so_by_the_cell_rather_than_timing_out TestCell.boot(concurrency: 1, queue_size: 4, queue_wait: 0.3, deadline: 30) do |cell| diff --git a/hotcell-server/test/support/test_cell.rb b/hotcell-server/test/support/test_cell.rb index 8ea1e1a..a1021ba 100644 --- a/hotcell-server/test/support/test_cell.rb +++ b/hotcell-server/test/support/test_cell.rb @@ -30,7 +30,7 @@ def send_line(line, descriptors: [], timeout: 10) begin connection.send_message line, descriptors: descriptors # A cell that refuses without reading writes its answer and closes first, so a send that loses that - # race fails with the verdict already queued on the socket. The verdict is what the suite asserts + # race fails with the answer already queued on the socket. The answer is what the suite asserts # on, so read it rather than surfacing the failed write. rescue SystemCallError, IOError nil diff --git a/hotcell-server/test/test_cell_test.rb b/hotcell-server/test/test_cell_test.rb index 51df3d3..00f5030 100644 --- a/hotcell-server/test/test_cell_test.rb +++ b/hotcell-server/test/test_cell_test.rb @@ -4,7 +4,7 @@ # The suite's own harness. A cell that refuses a connection never reads it: accept_work writes the # capacity answer and closes. A test client descheduled between connecting and writing then fails its -# send against the closed peer — with the verdict it came for already queued on its socket. +# send against the closed peer — with the answer it came for already queued on its socket. class TestCellTest < HotCellServerTest # Yields a connection the test prepared instead of dialing a cell, which pins the losing side of the # race: the peer has answered and closed before send_line writes a byte. @@ -16,7 +16,7 @@ def connect(_name = "work.sock") end end - def test_send_line_returns_a_verdict_that_arrived_before_the_request_was_written + def test_send_line_returns_an_answer_that_arrived_before_the_request_was_written cell_side, caller_side = UNIXSocket.pair(:STREAM) cell_side.write HotCell::Response.failed(HotCell::Failure.new(code: "capacity", message: "the queue is full at 0")).to_line diff --git a/hotcell-server/test/worker_reports_test.rb b/hotcell-server/test/worker_reports_test.rb index c88b5a9..ee485a0 100644 --- a/hotcell-server/test/worker_reports_test.rb +++ b/hotcell-server/test/worker_reports_test.rb @@ -137,7 +137,7 @@ def test_unknown_idle_codes_do_not_grow_the_counters_without_bound assert_equal [ :total, :unknown ].sort, @supervisor.counters.to_h[:requests].keys.sort end - # A code this cell actually mints is recorded as itself, so the sanitizing does not flatten real outcomes. + # A code this cell actually mints is recorded as itself, so the sanitizing does not flatten real codes. def test_a_known_idle_code_is_recorded_as_itself make_busy apply({ idle: true, code: "failed" }.to_json) diff --git a/hotcell-server/test/worker_test.rb b/hotcell-server/test/worker_test.rb index a42397d..a4b27ef 100644 --- a/hotcell-server/test/worker_test.rb +++ b/hotcell-server/test/worker_test.rb @@ -28,7 +28,7 @@ def test_a_connection_that_closes_before_a_request_is_reported_unavailable assert_equal({ idle: true, code: "unavailable" }, report) end - # A home the worker cannot create is a broken deployment, not a verdict on the document — and a request + # A home the worker cannot create is a broken deployment, not a permanent failure of the document — and a request # that never ran must not be reported as one that succeeded. Creating it used to sit outside the begin, so # the failure left the caller with no response while the supervisor was told idle `"ok"`. def test_a_home_that_cannot_be_created_is_reported_failed diff --git a/yabeda-hotcell/lib/yabeda/hot_cell.rb b/yabeda-hotcell/lib/yabeda/hot_cell.rb index 091f662..33c3029 100644 --- a/yabeda-hotcell/lib/yabeda/hot_cell.rb +++ b/yabeda-hotcell/lib/yabeda/hot_cell.rb @@ -18,7 +18,7 @@ def self.install! Yabeda.configure do group :hotcell - counter :requests, comment: "Calls through perform_in_hotcell, by outcome", + counter :requests, comment: "Calls through perform_in_hotcell, by code", tags: %i[ cell operation code cause ] histogram :perform, comment: "Time the cell spent performing", unit: :seconds, tags: %i[ cell operation ], buckets: [ 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, 30, 120 ] diff --git a/yabeda-hotcell/test/perform_test.rb b/yabeda-hotcell/test/perform_test.rb index 919b7fe..db4a8b9 100644 --- a/yabeda-hotcell/test/perform_test.rb +++ b/yabeda-hotcell/test/perform_test.rb @@ -28,7 +28,7 @@ def test_records_no_perform_time_when_the_cell_refuses_a_call_with_capacity assert_nil perform_seconds end - # `killed` is one code and several verdicts: which limit the worker hit decides whether the file did it. + # `killed` is one code and several causes: which limit the worker hit decides whether the file did it. # Every other code carries an empty cause, because a label that is sometimes absent is a separate series in # Prometheus, and a query by code would silently split. def test_counts_a_kill_under_its_cause From 26f8c7bee945dbf768c6f7023f164a8184cba490 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 9 Oct 2026 15:21:23 -0400 Subject: [PATCH 4/4] Add CHANGELOG entries and upgrade steps for the error vocabulary --- CHANGELOG.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 631b0d4..33a2710 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,16 +15,41 @@ gem but are what an operator runs against their own image. ## next / unreleased +### Upgrading + +Some actions that application developers should consider taking when upgrading from an earlier version: + +* Replace `rescue HotCell::Verdict` with `rescue HotCell::Failure::Exception`, and `HotCell::Client::Legacy::Verdict` with `HotCell::Client::Legacy::Failure::Exception`. Only the name changed: the exception still carries `hot_cell_failure`. +* In your own `perform.hot_cell` subscribers, test for success with `payload[:code] == "ok"` instead of `payload[:code].nil?`, and expect `interrupted`. The event now sets `code` on every call; see [Per-call notification](docs/observability.md#per-call-notification). +* Where a test instruments `perform.hot_cell` by hand to stand in for a successful call, pass `code: "ok"`. `HotCell::LogSubscriber` and `yabeda-hotcell` now record `code` as the event gives it, so an event without one records no code. +* Count `code="interrupted"` as a failed call wherever a dashboard or alert computes an error rate from `hotcell_requests`. `yabeda-hotcell` counted these calls as `ok` before. + ### HotCell::Client +#### Breaking + +* The `perform.hot_cell` event sets `code` on every call: `ok` on success, the failure's code on failure, and `interrupted` when an exception interrupts the call, such as the application's own request timeout. Previously, `code` was `nil` on success and absent from an interrupted call, so each subscriber had to work out the call's result. (#93) +* `HotCell::Verdict` is renamed `HotCell::Failure::Exception`. + +#### Changed + +* `HotCell::LogSubscriber` writes `"code":"interrupted"` for a call that an exception interrupted, beside the exception's class. Previously, the line had no code. + #### Fixed * On macOS, a call to a full cell reports `capacity` instead of occasionally reporting `unavailable`. A full cell answers and closes the connection without reading the request, so the client's send fails. The client already ignored that failure as `EPIPE` or `ECONNRESET` and read the answer, but macOS sometimes raises `ENOTCONN` instead. (#110) +### HotCell::Client::Legacy + +#### Breaking + +* `HotCell::Client::Legacy::Verdict` is renamed `HotCell::Client::Legacy::Failure::Exception`. + ### Yabeda::HotCell #### Fixed +* A call that an exception interrupted, such as the application's own request timeout, is counted in `requests` under `code="interrupted"` and records no `perform` observation. Previously, it was counted as `code="ok"` with a `perform` of 0 seconds. (#93) * `perform` records nothing for a call whose response has no `perform_ms`: a call that the cell refused with `capacity`, or that the client failed itself with `unavailable` or `timeout`. Previously, it recorded 0 seconds, which pulled down the low percentiles. (#112) ## v1.1.0 / 2026-10-08