Reduce the scope of the illegal call tracer and other tracing fixes#332
Reduce the scope of the illegal call tracer and other tracing fixes#332cretz merged 5 commits intotemporalio:mainfrom
Conversation
|
Converting to draft while we investigate why |
b77f223 to
6ebf37e
Compare
There was a problem hiding this comment.
These utilities are here just to help display/debug GC issues if there is a failure in the GC test
6ebf37e to
846e5d5
Compare
| parent = {} # child_id -> parent_id | ||
| root_of = {} # obj_id -> root_category | ||
|
|
||
| roots.each do |category, objs| |
There was a problem hiding this comment.
Can't believe you had to write a graph search lol
There was a problem hiding this comment.
AI did it, not me 😁. I'd never devote this kind of time to a test utility only invoked on failure, but I figured since I used it and confirmed it's a good utility, I might as well keep it around and use it on failure. I'd also never write such difficult-to-comprehend code, I'd have more human friendliness in lots of aspects.
| end | ||
| end | ||
|
|
||
| def test_many_children |
There was a problem hiding this comment.
Would this trigger the detector before?
There was a problem hiding this comment.
In slower machines like some CI runners, yes. In faster machines, COUNT needs to be 1000.
|
Ug, getting GC fail on 3.4 sometimes, and getting server-caused hanging on repeated |
177d7d5 to
20c59ba
Compare
20c59ba to
3660e90
Compare
|
I will be handling GC issues separately in #334. I skipped the test and this PR is now good, will merge once CI done. |
* envconfig impl * linting * fix steep type checks * formatting * Refactor envconfig classes to use Data.define for immutability - Refactor ClientConfigTLS, ClientConfigProfile, and ClientConfig to use Data.define * Rename hash methods to use idiomatic Ruby naming - Rename from_hash → from_h for all envconfig classes - Rename to_hash → to_h for all envconfig classes * Replace TLS config hash with Connection::TLSOptions object - Rename `to_connect_tls_config` → `to_tls_options` - Return `Connection::TLSOptions` object instead of plain Hash * Refactor to use inline pattern instead of temporary variables * Refactor to_client_connect_options to return tuple for one-liner usage Changed `to_client_connect_options` to return `[positional_args, keyword_args]` tuple instead of a hash, enabling clean one-liner client connections: ```ruby args, kwargs = profile.to_client_connect_options client = Temporalio::Client.connect(*args, **kwargs) ``` * Make hash methods private * Remove File.exist? check by using type system for path vs data distinction - TOML paths (*_path fields) become Pathname objects - TOML data (*_data fields) remain String objects * Use symbol keys instead of string keys in Rust FFI bridge Changed all hash keys from strings to symbols in the Rust-to-Ruby bridge * Bump golang.org/x/net in /temporalio/test/golangworker (#303) * Bump tracing-subscriber from 0.3.19 to 0.3.20 in /temporalio (#331) * Reduce the scope of the illegal call tracer and other tracing fixes (#332) * Remove unnecessary fully qualified import names * Remove envconfig import in temporalio.rb * _source_to_path_and_data prefixed with underscore and private doc visibility * TLS disabled field now optional * Rubocop fixes * Steep fixes * Renamed envconfig.rb -> env_config.rb Renamed to_tls_options -> to_client_tls_options * Pass data string as RString, Rust bridge converts to Vec * Rename load_client_connect_config -> load_client_connect_options * Format envconfig.rs * Nits * Update core submodule * Revert irrelevant cancellation changes * Update Cargo.lock * Propagate set_headers failure in client bridge * Add experimental warning --------- Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chad Retz <[email protected]>
What was changed
Deadlock detector was tripping on 1000 child workflows at once (see #330). This was because our illegal call tracer can be expensive. We made the following improvements:
run_until_all_yieldedcode which allows us to save many calls to theTracePointTracePoint#disablewhich was showing issues of not being reentrantworkflow_instance.rbto no longer delegate two methods deep to apply activation (helps stack traces and code legibility)Checklist