Add standalone-activity support to throughput_stress (RE-353)#403
Conversation
|
Verification run (local, server 1.31.0 via Scenario completed cleanly, and One caveat: Also pushed: regenerated Ruby kitchen-sink bindings ( |
ecb0d9d to
9c55cd1
Compare
|
Reworked this PR to fold in the advantages of #351 (thanks @prathyushpv) while keeping this PR's What changed from the original version of this PR:
Verified builds: Go ( Still outstanding before un-drafting: not yet exercised against a live server with |
9099991 to
1be8737
Compare
THardy98
left a comment
There was a problem hiding this comment.
Couple small things but Ruby support is blocking
| throw new ApplicationFailureException( | ||
| "DoStandaloneNexusOperation is not supported", "UnsupportedOperation", nonRetryable: true); | ||
| } | ||
| else if (action.DoStandaloneActivity != null) |
There was a problem hiding this comment.
do we also need to check if this is explicitly false
There was a problem hiding this comment.
DoStandaloneActivity is a message-typed oneof member, so the generated property is the message or null — != null is the "is this variant set" check (there's no false/default case to guard as there would be for a bool), and it matches every other branch in this method.
Basically, this isn't a bool, it is either null, or a valid value
There was a problem hiding this comment.
Just as an aside - it's a bit odd that this file is located in loadgen/kitchensink, theClientActionsExecutor is worker logic, it doesn't generate any load.
(not really related to your PR, but just noting it)
| when :do_standalone_activity | ||
| raise Temporalio::Error::ApplicationError.new( | ||
| 'DoStandaloneActivity is not supported', | ||
| non_retryable: true | ||
| ) |
There was a problem hiding this comment.
standalone activities is supported in ruby SDK version 1.5.0 (see temporalio/sdk-ruby#443)
in any case, if there's a feature that's not supported by a particular SDK, we should check err-if-unimplemented here and log instead of throwing if set
There was a problem hiding this comment.
Added the checks for err-if-unimplemented
There was a problem hiding this comment.
any reason why we didn't add Ruby support ?
There was a problem hiding this comment.
The reason it was stubbed: the Ruby worker's Gemfile.lock/gemspec were pinned to temporalio 1.3.0, which predates the standalone-activity API. But that was actually a latent inconsistency from #404 — that PR bumped versions.env to RUBY_SDK_VERSION=1.5.0 but left the lock/gemspec at 1.3.0.
Adding it now
Adds an include-standalone-activity scenario option that starts activities outside any workflow context via StartActivityExecution and waits for the outcome with PollActivityExecution, mirroring the include-standalone-nexus pattern (#339) and bench-go's standalone-activity throughputstress step (bench-go #357/#358). - kitchen_sink.proto: new ClientAction variant DoStandaloneActivity. - Go client-action executor: Start + Poll implementation; targets the existing noop activity on the workflow's task queue. - Other SDK workers raise UnsupportedOperation, like standalone Nexus. - Bindings regenerated via 'go run ./cmd/dev build-proto' (Go, Java, Python, .NET; Ruby pb not covered by build-proto - see PR notes). Requires server-side activity.enableStandalone.
protoc --ruby_out with the same require rewrite as the existing file (temporal/api/*_pb -> temporalio/api/*); build-proto does not cover Ruby.
The previous regen used a local protoc 7.x, producing gencode for protobuf runtimes newer than the repo pins (Java RuntimeVersion/ GeneratedFile, .NET end-group handling, protoc version headers) - which broke compilation against the pinned runtimes and failed the protos-up-to-date check. Rebuilt via 'dev install protoc' + 'dev build-proto' exactly as CI does. Ruby is intentionally untouched: CI does not regenerate it and its committed style matches main.
Integrates the advantages of #351 into this PR while keeping the include-standalone-activity flag and README conventions: - proto: DoStandaloneActivity now carries ExecuteActivityAction (was a bare activity_type string), so the activity variant, task queue, timeouts, and retry policy are all configurable. Regenerated Go/Java/Python/.NET/Ruby bindings (TS protos regenerate at build). - Go loadgen executor uses the high-level SDK client.ExecuteActivity + StartActivityOptions instead of raw StartActivityExecution/PollActivityExecution RPCs; drops manual Namespace threading. - Extracted ActivityNameAndArgs + ConvertFromPBRetryPolicy into shared helpers and deduped the Go worker's launchActivity to reuse them. - throughput_stress emits a payload(256B/256B) standalone activity with configurable timeouts/retry instead of noop. - Implemented the standalone-activity client action for Python, TypeScript, .NET, and Java workers (each via its high-level SDK API + a shared ActivityDispatch helper). Ruby remains stubbed. - Bumped Java SDK to 1.35.0 (required for ActivityClient/StartActivityOptions). Builds verified: Go (build+test), Python (mypy), TS (tsc), .NET (dotnet build), Java (gradle build + Spotless).
include-standalone-nexus and include-standalone-activity are parsed as free-form --option flags, so they don't appear in cobra --help. The scenario Description string is the only place omes list-scenarios can surface them, so add both.
11ffb2d to
1bfc3f8
Compare
The standalone-nexus stubs (all languages) and Ruby's standalone-activity stub raised unconditionally, ignoring the err-on-unimplemented flag. Make them follow the same pattern as concurrent client actions: raise only when err-on-unimplemented is set, otherwise log and skip.
The Go executor errors when DoStandaloneActivity.activity is unset; the other workers either NPE'd or silently ran a noop activity. Add the same guard to the .NET, Java, Python, and TypeScript workers so a missing activity fails loudly and consistently across languages.
The standalone-nexus stubs now respect err-on-unimplemented, so they skip silently by default instead of throwing. The kitchensink test that asserts a worker rejects an unsupported feature relied on the unconditional throw, so thread --err-on-unimplemented through the test harness and have testUnsupportedFeature start the worker in strict mode. This preserves the rejection assertion while keeping the gated skip-by-default behavior.
The Ruby SDK gained standalone-activity client APIs in 1.5.0 (temporalio/sdk-ruby#443). Bump the Ruby SDK from 1.3.0 to 1.5.0 (versions.env already targeted 1.5.0; the Gemfile.lock and gemspec lagged) and implement DoStandaloneActivity via client.execute_activity, mirroring the other workers. Extract a shared ActivityDispatch helper so the workflow-scheduled and standalone-activity paths stay in sync.
Exercises DoStandaloneActivity end-to-end across all SDKs (the prior suite only covered standalone Nexus). The client activity starts a standalone activity on the run's task queue and waits for it; the worker picks it up and completes it. Enables activity.enableStandalone dynamic config and fills the activity's task queue per run.
After extracting the activity mapping into ActivityDispatch, launch_activity is small enough that the Metrics cops no longer fire, so the rubocop:disable/enable Metrics directives are flagged as redundant by check-worker. Remove them.
THardy98
left a comment
There was a problem hiding this comment.
Would prefer if we could add
What was changed
Adds an
include-standalone-activityscenario option tothroughput_stressthat starts activities outside any workflow context viaStartActivityExecutionand waits for the outcome withPollActivityExecution— mirroring theinclude-standalone-nexuspattern (#339) and bench-go's standalone-activity throughputstress step (bench-go #357/#358).kitchen_sink.proto: newClientActionvariantDoStandaloneActivity(field 7).noopactivity on the workflow's task queue. Runs for both the loadgen runner and the Go worker (shared executor).UnsupportedOperation, exactly like standalone Nexus.go run ./cmd/dev build-proto(Go, Java, Python, .NET).Why
bench-go grew an
EnableStandaloneActivitytoggle (used by saas-cicd chasm e2e pipelines, with server dynamic configactivity.enableStandalone) after the bench→omes parity scan closed. omes had no standalone-activity coverage at all — this closes that gap so the saas-cicd e2e cutover (RE-264) isn't blocked on it. RE-353, part of epic RE-257.Notes for review
workers/ruby/.../kitchen_sink_pb.rb) is not covered bydev build-proto— if it's generated by a separate step, it needs a follow-up regen (the Ruby executor stub is in place).bdd2879), which I deliberately did not replicate: the option is opt-in, so failing loudly on unsupported servers seems preferable. Open to changing.go build,go test ./scenarios/ ./loadgen/kitchensink/pass. Not yet exercised against a server withactivity.enableStandalone— needs a verification run before un-drafting.