Skip to content

Conversation

erdirowlands
Copy link
Contributor

@erdirowlands erdirowlands commented Nov 18, 2024

What

Concurrency Changes

  • Replaces concurrent-ruby/ConcurrentMap with mutexes to resolve a segmentation fault that is seen under difficult to reproduce circumstances.
  • Performs deep cloning of metrics maps using Marshal.load/Marshal.dump.
  • Uses a new mutex for when we aggregate and send data at the end an interval
  • Uses a Set for seen targets to improve performance + places hard limit of 500k targets to stop unbounded growth.
  • Decreases thread pool in the stream processor (update processor) from 100 to 20. 100 threads was excessive. Even if someone is toggling flags and target groups every second, 20 threads is still more than enough to process the work.
  • Uses a Condition instead of a "dumb sleep" for wait_for_initialization and timeout usecases

Testing

  • New multi-threaded unit tests
  • Testgrid
  • Manual testing

Evaluator changes

  • Now logs if a flag is not found in the cache and continues to return the default variation
  • Now logs if the variation requested doesn't match the flag type, and returns the default variation early

Dependencies

  • Bumps murmurhash3 to 0.1.7

Performance considerations

  • The deep clone can introduce overhead, but as this is only ran once per minute at a minimum when we agg + send metrics, the overhead should be negligible compared to the immutability benefits.

@feature_name_attribute = "featureName"
@variation_identifier_attribute = "variationIdentifier"

@executor = Concurrent::FixedThreadPool.new(10)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not being used so safe to remove.

@erdirowlands erdirowlands merged commit c5053ce into main Nov 18, 2024
1 check passed
@erdirowlands erdirowlands deleted the FFM-12192-patch-4 branch November 18, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants