Skip to content

fix: use sync pool for strategy randomness - #212

Merged
sighphyre merged 3 commits into
chore/public-api-works-on-atomic-pointersfrom
fix/use-sync-pool-for-strategy-randomness
Nov 21, 2025
Merged

fix: use sync pool for strategy randomness#212
sighphyre merged 3 commits into
chore/public-api-works-on-atomic-pointersfrom
fix/use-sync-pool-for-strategy-randomness

Conversation

@sighphyre

@sighphyre sighphyre commented Nov 18, 2025

Copy link
Copy Markdown
Member

Instead of blocking on a single shared Mutex, randomness now uses a sync pool, which gives multiple goroutines a bit more breathing room to get access to randomness without lining up one behind another

@sighphyre
sighphyre changed the base branch from v5 to chore/public-api-works-on-atomic-pointers November 18, 2025 15:31
@sighphyre
sighphyre force-pushed the fix/use-sync-pool-for-strategy-randomness branch from d5f59a3 to a0692e0 Compare November 19, 2025 07:28
@sighphyre sighphyre self-assigned this Nov 19, 2025
@coveralls

coveralls commented Nov 19, 2025

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 19563565440

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 84 of 127 (66.14%) changed or added relevant lines in 6 files are covered.
  • 67 unchanged lines in 7 files lost coverage.
  • Overall coverage decreased (-4.5%) to 77.151%

Changes Missing Coverage Covered Lines Changed/Added Lines %
api/feature.go 4 6 66.67%
internal/strategies/flexible_rollout.go 0 2 0.0%
internal/strategies/helpers.go 0 10 0.0%
metrics.go 72 101 71.29%
Files with Coverage Reduction New Missed Lines %
nooplistener.go 2 62.5%
internal/api/metrics.go 3 0.0%
debuglistener.go 6 0.0%
unleash_mock.go 6 75.0%
client.go 7 86.93%
feature_state.go 20 59.29%
metrics.go 23 75.37%
Totals Coverage Status
Change from base Build 19538258348: -4.5%
Covered Lines: 1587
Relevant Lines: 2057

💛 - Coveralls

@sighphyre
sighphyre force-pushed the fix/use-sync-pool-for-strategy-randomness branch from a0692e0 to db94232 Compare November 19, 2025 08:25
@chriswk chriswk moved this from New to In Progress in Issues and PRs Nov 19, 2025
@sighphyre
sighphyre marked this pull request as ready for review November 19, 2025 09:10

@FredrikOseberg FredrikOseberg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks sane. Nice! Also this is backwards compatible so we can merge it towards both branches.

EDIT: Is the target branch here correct Simon? I would imagine you want to merge this into v5 or v6.

@github-project-automation github-project-automation Bot moved this from In Progress to Approved PRs in Issues and PRs Nov 19, 2025
@sighphyre
sighphyre force-pushed the chore/public-api-works-on-atomic-pointers branch from 4b5508d to cc57f12 Compare November 20, 2025 12:41
@sighphyre
sighphyre force-pushed the fix/use-sync-pool-for-strategy-randomness branch from db94232 to e39f515 Compare November 20, 2025 12:49
@sighphyre
sighphyre force-pushed the fix/use-sync-pool-for-strategy-randomness branch from e39f515 to 1c38a81 Compare November 20, 2025 13:21
@sighphyre

sighphyre commented Nov 20, 2025

Copy link
Copy Markdown
Member Author

This looks sane. Nice! Also this is backwards compatible so we can merge it towards both branches.

EDIT: Is the target branch here correct Simon? I would imagine you want to merge this into v5 or v6.

Yessir, it's correct. This is a stacked PR, if you're not familiar with the flow, it's worth doing some reading on it because it's a hella useful flow for this kind of situation. This entire chain builds on top of itself so this entire set of PRs require the work before them. Stacking the PR lets me work without being blocked by review and prevents you from having to review a 4k +/- monstrosity

The base now points to v6, so as these merge they'll all fall into that

* chore: rework metrics to use less locking on the hotpath

* fix: variant strategies now correctly inherit stickiness from their strategy

* chore: make metric count channels best effort send
@sighphyre
sighphyre merged commit f13456f into chore/public-api-works-on-atomic-pointers Nov 21, 2025
4 checks passed
@sighphyre
sighphyre deleted the fix/use-sync-pool-for-strategy-randomness branch November 21, 2025 07:42
@github-project-automation github-project-automation Bot moved this from Approved PRs to Done in Issues and PRs Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants