temporalio / temporalio/temporal
Fairsim partial counter configuration resets unspecified defaults
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 23.2k
- Forks
- 1.9k
- Avg merge
- 2d 8h
- Merged PRs (30d)
- 228
Description
Expected Behavior
The partial counter configuration documented in tools/fairsim/README.md should override only the supplied values.
For example:
./fairsim \
-counter-params=<(echo '{"CMS":{"W":100}}') \
-- -tasks=1000 -keys=500
should set the CMS width to 100 while retaining all other values from counter.DefaultCounterParams.
Actual Behavior
fairsim unmarshals the JSON into a zero-valued counter.CounterParams. Omitted values such as MapLimit, CMS depth, growth settings, and reseeding interval therefore become zero.
The loaded configuration includes:
MapLimit: 0
CMS.W: 100
CMS.D: 0
CMS.Grow: zero values
CMS.Reseed.Interval: 0
With MapLimit set to zero, processing the first task can panic in mapCounter.updateHeap. It also means that changing one CMS setting unintentionally changes the rest of the simulated counter configuration.
Steps to Reproduce the Problem
-
Build
fairsimfrom an unmodified checkout:make fairsim -
Run the partial configuration example:
./fairsim \ -counter-params=<(echo '{"CMS":{"W":100}}') \ -- -tasks=1000 -keys=500 -
Observe that unspecified counter parameters are zero instead of retaining their defaults, potentially followed by an index-out-of-range panic.
Proposed Resolution
Initialize the configuration with counter.DefaultCounterParams before unmarshalling the supplied JSON. This makes partial configuration files behave as documented while continuing to support explicit zero values.
A regression test can load {"CMS":{"W":10}} and verify that only CMS.W changes while all other fields remain equal to counter.DefaultCounterParams.
Specifications
- Version:
mainat632d694e7 - Platform: Linux amd64
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with tools/fairsim/README.md and the fairsim configuration loading path, then run make fairsim and the partial CMS.W example. Compare the loaded values with counter.DefaultCounterParams and add the proposed regression test for {"CMS":{"W":10}}. Done means CMS.W is overridden while unspecified counter fields retain their defaults, including explicit zero values.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100