From 24705bf95fbf4db19fd3bdb630b719ba1f1fd890 Mon Sep 17 00:00:00 2001 From: Chad Retz Date: Tue, 21 Jan 2025 15:03:05 -0600 Subject: [PATCH] Remove Ractor expectations and enable Linux ARM in CI --- .github/workflows/ci.yml | 14 ++-- README.md | 18 ++--- .../temporalio/converters/data_converter.rb | 11 ++- .../converters/failure_converter.rb | 2 +- .../converters/payload_converter.rb | 14 ++-- temporalio/lib/temporalio/worker.rb | 5 +- .../temporalio/worker/workflow_executor.rb | 3 +- .../worker/workflow_executor/ractor.rb | 69 ------------------- .../worker/workflow_executor/ractor.rbs | 9 --- .../test/converters/data_converter_test.rb | 4 +- .../test/testing/workflow_environment_test.rb | 12 +--- temporalio/test/worker_workflow_test.rb | 13 +--- temporalio/test/workflow_utils.rb | 2 - 13 files changed, 35 insertions(+), 141 deletions(-) delete mode 100644 temporalio/lib/temporalio/worker/workflow_executor/ractor.rb delete mode 100644 temporalio/sig/temporalio/worker/workflow_executor/ractor.rbs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7cec288f..ba47f9cd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,15 +11,9 @@ jobs: strategy: fail-fast: false matrix: - # TODO(cretz): Enable Linux ARM. It's not natively supported with setup-ruby (see - # https://github.com/ruby/setup-ruby#supported-platforms and https://github.com/ruby/setup-ruby/issues/577). - # So we need to set ruby-version to 'none' per - # https://github.com/oxidize-rb/actions/tree/main/setup-ruby-and-rust and install Ruby ourselves maybe. See - # https://github.com/ruby/setup-ruby?tab=readme-ov-file#using-self-hosted-runners. The error states: - # Error: The current runner (ubuntu-24.04-arm64) was detected as self-hosted because the platform does not match a GitHub-hosted runner image (or that image is deprecated and no longer supported). - # In such a case, you should install Ruby in the $RUNNER_TOOL_CACHE yourself, for example using https://github.com/rbenv/ruby-build - # You can take inspiration from this workflow for more details: https://github.com/ruby/ruby-builder/blob/master/.github/workflows/build.yml - # + # TODO(cretz): Enable ubuntu-24.04-arm when oxidize-rb/actions/setup-ruby-and-rust + # supports it + # # TODO(cretz): Enable x64-mingw-ucrt if we can figure out Windows issue, see # https://github.com/temporalio/sdk-ruby/issues/172 os: [ubuntu-latest, macos-latest] @@ -28,7 +22,7 @@ jobs: include: - os: ubuntu-latest - rubyVersion: "3.3" + rubyVersion: "3.4" checkTarget: true runs-on: ${{ matrix.os }} steps: diff --git a/README.md b/README.md index 43c107ea..6523f242 100644 --- a/README.md +++ b/README.md @@ -152,9 +152,7 @@ worker = Temporalio::Worker.new( task_queue: 'my-task-queue', workflows: [SayHelloWorkflow], # There are various forms an activity can take, see "Activities" section for details - activities: [SayHelloActivity], - # During the beta period, this must be provided explicitly, see "Workers" section for details - workflow_executor: Temporalio::Worker::WorkflowExecutor::ThreadPool.default + activities: [SayHelloActivity] ) # Run the worker until SIGINT. This can be done in many ways, see "Workers" section for details. @@ -357,9 +355,7 @@ worker = Temporalio::Worker.new( task_queue: 'my-task-queue', workflows: [MyModule::MyWorkflow], # There are various forms an activity can take, see "Activities" section for details - activities: [MyModule::MyActivity], - # During the beta period, this must be provided explicitly, see below for details - workflow_executor: Temporalio::Worker::WorkflowExecutor::ThreadPool.default + activities: [MyModule::MyActivity] ) # Run the worker until block complete @@ -372,9 +368,6 @@ Notes about the above code: * A worker uses the same client that is used for other Temporal things. * This just shows providing an activity class, but there are other forms, see the "Activities" section for details. -* The `workflow_executor` defaults to `Temporalio::Worker::WorkflowExecutor::Ractor.instance` which intentionally does - not work during this beta period. Therefore, during this beta period, opting in to - `Temporalio::Worker::WorkflowExecutor::ThreadPool.default` is required explicitly. * The worker `run` method accepts an optional `Temporalio::Cancellation` object that can be used to cancel instead or in addition to providing a block that waits for completion. * The worker `run` method accepts a `shutdown_signals` array which will trap the signal and start shutdown when @@ -994,6 +987,13 @@ it will raise the error raised in the activity. The constructor of the environment has multiple keyword arguments that can be set to affect the activity context for the activity. +### Ractors + +It was an original goal to have workflows actually be Ractors for deterministic state isolation and have the library +support Ractors in general. However, due to the SDK's heavy use of the Google Protobuf library which +[is not Ractor-safe](https://github.com/protocolbuffers/protobuf/issues/19321), the Temporal Ruby SDK does not currently +work with Ractors. + ### Platform Support This SDK is backed by a Ruby C extension written in Rust leveraging the diff --git a/temporalio/lib/temporalio/converters/data_converter.rb b/temporalio/lib/temporalio/converters/data_converter.rb index 322f3a81..f518ac7e 100644 --- a/temporalio/lib/temporalio/converters/data_converter.rb +++ b/temporalio/lib/temporalio/converters/data_converter.rb @@ -8,10 +8,10 @@ module Temporalio module Converters # Data converter for converting/encoding payloads to/from Ruby values. class DataConverter - # @return [PayloadConverter] Payload converter. This must be Ractor shareable. + # @return [PayloadConverter] Payload converter. attr_reader :payload_converter - # @return [FailureConverter] Failure converter. This must be Ractor shareable. + # @return [FailureConverter] Failure converter. attr_reader :failure_converter # @return [PayloadCodec, nil] Optional codec for encoding/decoding payload bytes such as for encryption. @@ -24,17 +24,14 @@ def self.default # Create data converter. # - # @param payload_converter [PayloadConverter] Payload converter to use. This must be Ractor shareable. - # @param failure_converter [FailureConverter] Failure converter to use. This must be Ractor shareable. + # @param payload_converter [PayloadConverter] Payload converter to use. + # @param failure_converter [FailureConverter] Failure converter to use. # @param payload_codec [PayloadCodec, nil] Payload codec to use. def initialize( payload_converter: PayloadConverter.default, failure_converter: FailureConverter.default, payload_codec: nil ) - raise 'Payload converter not shareable' unless Ractor.shareable?(payload_converter) - raise 'Failure converter not shareable' unless Ractor.shareable?(failure_converter) - @payload_converter = payload_converter @failure_converter = failure_converter @payload_codec = payload_codec diff --git a/temporalio/lib/temporalio/converters/failure_converter.rb b/temporalio/lib/temporalio/converters/failure_converter.rb index c2c78afe..b2c6d7b6 100644 --- a/temporalio/lib/temporalio/converters/failure_converter.rb +++ b/temporalio/lib/temporalio/converters/failure_converter.rb @@ -10,7 +10,7 @@ module Converters class FailureConverter # @return [FailureConverter] Default failure converter. def self.default - @default ||= Ractor.make_shareable(FailureConverter.new) + @default ||= FailureConverter.new end # @return [Boolean] If +true+, the message and stack trace of the failure will be moved into the encoded attribute diff --git a/temporalio/lib/temporalio/converters/payload_converter.rb b/temporalio/lib/temporalio/converters/payload_converter.rb index 5a2ca2aa..81c680ec 100644 --- a/temporalio/lib/temporalio/converters/payload_converter.rb +++ b/temporalio/lib/temporalio/converters/payload_converter.rb @@ -22,14 +22,12 @@ def self.default # @param json_generate_options [Hash] Options for {::JSON.generate}. # @return [PayloadConverter::Composite] Created payload converter. def self.new_with_defaults(json_parse_options: { create_additions: true }, json_generate_options: {}) - Ractor.make_shareable( - PayloadConverter::Composite.new( - PayloadConverter::BinaryNull.new, - PayloadConverter::BinaryPlain.new, - PayloadConverter::JSONProtobuf.new, - PayloadConverter::BinaryProtobuf.new, - PayloadConverter::JSONPlain.new(parse_options: json_parse_options, generate_options: json_generate_options) - ) + PayloadConverter::Composite.new( + PayloadConverter::BinaryNull.new, + PayloadConverter::BinaryPlain.new, + PayloadConverter::JSONProtobuf.new, + PayloadConverter::BinaryProtobuf.new, + PayloadConverter::JSONPlain.new(parse_options: json_parse_options, generate_options: json_generate_options) ) end diff --git a/temporalio/lib/temporalio/worker.rb b/temporalio/lib/temporalio/worker.rb index f19c30d4..b3ca0ced 100644 --- a/temporalio/lib/temporalio/worker.rb +++ b/temporalio/lib/temporalio/worker.rb @@ -286,7 +286,8 @@ def self.default_illegal_workflow_calls # @param workflows [Array>] Workflows for this worker. # @param tuner [Tuner] Tuner that controls the amount of concurrent activities/workflows that run at a time. # @param activity_executors [Hash] Executors that activities can run within. - # @param workflow_executor [WorkflowExecutor] Workflow executor that workflow tasks run within. + # @param workflow_executor [WorkflowExecutor] Workflow executor that workflow tasks run within. This must be a + # {WorkflowExecutor::ThreadPool} currently. # @param interceptors [Array] Interceptors specific to this worker. # Note, interceptors set on the client that include the {Interceptor::Activity} or {Interceptor::Workflow} module # are automatically included here, so no need to specify them again. @@ -352,7 +353,7 @@ def initialize( workflows: [], tuner: Tuner.create_fixed, activity_executors: ActivityExecutor.defaults, - workflow_executor: WorkflowExecutor::Ractor.instance, + workflow_executor: WorkflowExecutor::ThreadPool.default, interceptors: [], build_id: Worker.default_build_id, identity: nil, diff --git a/temporalio/lib/temporalio/worker/workflow_executor.rb b/temporalio/lib/temporalio/worker/workflow_executor.rb index b8cb48c1..77ce413c 100644 --- a/temporalio/lib/temporalio/worker/workflow_executor.rb +++ b/temporalio/lib/temporalio/worker/workflow_executor.rb @@ -1,12 +1,11 @@ # frozen_string_literal: true -require 'temporalio/worker/workflow_executor/ractor' require 'temporalio/worker/workflow_executor/thread_pool' module Temporalio class Worker # Workflow executor that executes workflow tasks. Unlike {ActivityExecutor}, this class is not meant for user - # implementation. Instead, either {WorkflowExecutor::ThreadPool} or {WorkflowExecutor::Ractor} should be used. + # implementation. The only implementation that is currently accepted is {WorkflowExecutor::ThreadPool}. class WorkflowExecutor # @!visibility private def initialize diff --git a/temporalio/lib/temporalio/worker/workflow_executor/ractor.rb b/temporalio/lib/temporalio/worker/workflow_executor/ractor.rb deleted file mode 100644 index 0301d6ef..00000000 --- a/temporalio/lib/temporalio/worker/workflow_executor/ractor.rb +++ /dev/null @@ -1,69 +0,0 @@ -# frozen_string_literal: true - -require 'temporalio/internal/worker/workflow_instance' -require 'temporalio/worker/workflow_executor' - -module Temporalio - class Worker - class WorkflowExecutor - # Ractor-based implementation of {WorkflowExecutor}. - # - # @note WARNING: This is not currently implemented. Do not try to use this class at this time. - class Ractor < WorkflowExecutor - include Singleton - - def initialize # rubocop:disable Lint/MissingSuper - # Do nothing - end - - # @!visibility private - def _validate_worker(_worker, _worker_state) - raise 'Ractor support is not currently working, please set ' \ - 'workflow_executor to Temporalio::Worker::WorkflowExecutor::ThreadPool' - end - - # @!visibility private - def _activate(activation, worker_state, &) - raise NotImplementedError - end - - # TODO(cretz): This does not work with Google Protobuf - # steep:ignore:start - - # @!visibility private - class Instance - def initialize(initial_details) - initial_details = ::Ractor.make_shareable(initial_details) - - @ractor = ::Ractor.new do - # Receive initial details and create the instance - details = ::Ractor.receive - instance = Internal::Worker::WorkflowInstance.new(details) - ::Ractor.yield - - # Now accept activations in a loop - loop do - activation = ::Ractor.receive - completion = instance.activate(activation) - ::Ractor.yield(completion) - end - end - - # Send initial details and wait until yielded - @ractor.send(initial_details) - @ractor.take - end - - # @!visibility private - def activate(activation) - @ractor.send(activation) - @ractor.take - end - end - - private_constant :Instance - # steep:ignore:end - end - end - end -end diff --git a/temporalio/sig/temporalio/worker/workflow_executor/ractor.rbs b/temporalio/sig/temporalio/worker/workflow_executor/ractor.rbs deleted file mode 100644 index 4f3c5dd7..00000000 --- a/temporalio/sig/temporalio/worker/workflow_executor/ractor.rbs +++ /dev/null @@ -1,9 +0,0 @@ -module Temporalio - class Worker - class WorkflowExecutor - class Ractor < WorkflowExecutor - def self.instance: -> Ractor - end - end - end -end \ No newline at end of file diff --git a/temporalio/test/converters/data_converter_test.rb b/temporalio/test/converters/data_converter_test.rb index 21106c1f..18b5ab9c 100644 --- a/temporalio/test/converters/data_converter_test.rb +++ b/temporalio/test/converters/data_converter_test.rb @@ -11,9 +11,7 @@ module Converters class DataConverterTest < Test def test_with_codec converter = Temporalio::Converters::DataConverter.new( - failure_converter: Ractor.make_shareable( - Temporalio::Converters::FailureConverter.new(encode_common_attributes: true) - ), + failure_converter: Temporalio::Converters::FailureConverter.new(encode_common_attributes: true), payload_codec: Base64Codec.new ) diff --git a/temporalio/test/testing/workflow_environment_test.rb b/temporalio/test/testing/workflow_environment_test.rb index 24472559..ec21238b 100644 --- a/temporalio/test/testing/workflow_environment_test.rb +++ b/temporalio/test/testing/workflow_environment_test.rb @@ -38,9 +38,7 @@ def test_time_skipping_auto worker = Temporalio::Worker.new( client: env.client, task_queue: "tq-#{SecureRandom.uuid}", - workflows: [SlowWorkflow], - # TODO(cretz): Ractor support not currently working - workflow_executor: Temporalio::Worker::WorkflowExecutor::ThreadPool.default + workflows: [SlowWorkflow] ) worker.run do # Check that timestamp is around now @@ -63,9 +61,7 @@ def test_time_skipping_manual worker = Temporalio::Worker.new( client: env.client, task_queue: "tq-#{SecureRandom.uuid}", - workflows: [SlowWorkflow], - # TODO(cretz): Ractor support not currently working - workflow_executor: Temporalio::Worker::WorkflowExecutor::ThreadPool.default + workflows: [SlowWorkflow] ) worker.run do # Start workflow @@ -121,9 +117,7 @@ def test_time_skipping_heartbeat_timeout client: env.client, task_queue: "tq-#{SecureRandom.uuid}", workflows: [HeartbeatTimeoutWorkflow], - activities: [HeartbeatTimeoutActivity.new(env)], - # TODO(cretz): Ractor support not currently working - workflow_executor: Temporalio::Worker::WorkflowExecutor::ThreadPool.default + activities: [HeartbeatTimeoutActivity.new(env)] ) worker.run do # Run workflow and confirm it got heartbeat timeout diff --git a/temporalio/test/worker_workflow_test.rb b/temporalio/test/worker_workflow_test.rb index ee017d4e..ff6491ca 100644 --- a/temporalio/test/worker_workflow_test.rb +++ b/temporalio/test/worker_workflow_test.rb @@ -1114,9 +1114,7 @@ def test_payload_codec # Workflow failure with failure encoding new_options = env.client.options.with( data_converter: Temporalio::Converters::DataConverter.new( - failure_converter: Ractor.make_shareable( - Temporalio::Converters::FailureConverter.new(encode_common_attributes: true) - ), + failure_converter: Temporalio::Converters::FailureConverter.new(encode_common_attributes: true), payload_codec: Base64Codec.new ) ) @@ -1164,9 +1162,7 @@ def test_dynamic worker = Temporalio::Worker.new( client: env.client, task_queue: "tq-#{SecureRandom.uuid}", - workflows: [DynamicWorkflow, NonDynamicWorkflow], - # TODO(cretz): Ractor support not currently working - workflow_executor: Temporalio::Worker::WorkflowExecutor::ThreadPool.default + workflows: [DynamicWorkflow, NonDynamicWorkflow] ) worker.run do # Non-dynamic @@ -1670,9 +1666,7 @@ def do_update(arg) def test_fail_workflow_payload_converter new_options = env.client.options.with( - data_converter: Temporalio::Converters::DataConverter.new( - payload_converter: Ractor.make_shareable(FailWorkflowPayloadConverter.new) - ) + data_converter: Temporalio::Converters::DataConverter.new(payload_converter: FailWorkflowPayloadConverter.new) ) client = Temporalio::Client.new(**new_options.to_h) @@ -1746,7 +1740,6 @@ def test_confirm_garbage_collect # TODO(cretz): To test # * Common - # * Ractor with global state # * Eager workflow start # * Unawaited futures that have exceptions, need to log warning like Java does # * Enhanced stack trace? diff --git a/temporalio/test/workflow_utils.rb b/temporalio/test/workflow_utils.rb index 382520ca..92d896d4 100644 --- a/temporalio/test/workflow_utils.rb +++ b/temporalio/test/workflow_utils.rb @@ -34,8 +34,6 @@ def execute_workflow( task_queue:, activities:, workflows: [workflow] + more_workflows, - # TODO(cretz): Ractor support not currently working - workflow_executor: Temporalio::Worker::WorkflowExecutor::ThreadPool.default, workflow_failure_exception_types:, max_cached_workflows:, logger: logger || client.options.logger,