Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Modify data kind constants to avoid Process.warmup rehash #301

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

keelerm84
Copy link
Member

The data stores in the SDK have to deal with multiple types of data --
features (or flags) and segments. These types have long been simple
hashes in our SDK, with one defining an optional lambda property.

With Ruby 3.3 and the introduction of Process.warmup, we have seen an
issue where, after warmup, the in memory store needs a rehash method
call before these kind constants can be used reliably to access the
store.

To combat this, I am moving these kinds into a class. This class as
explicit hash key behaviors, so theoretically shouldn't have this
problem Local testing has shown this to be the case.

This class is also given a dictionary interface to maintain compliance
with the existing implementation. While people should not be relying on
these constants explicitly, they do flow through the system in ways that
might make their signatures somewhat public.

The data stores in the SDK have to deal with multiple types of data --
features (or flags) and segments. These types have long been simple
hashes in our SDK, with one defining an optional lambda property.

With Ruby 3.3 and the introduction of `Process.warmup`, we have seen an
issue where, after warmup, the in memory store needs a `rehash` method
call before these kind constants can be used reliably to access the
store.

To combat this, I am moving these kinds into a class. This class as
explicit hash key behaviors, so theoretically shouldn't have this
problem Local testing has shown this to be the case.

This class is also given a dictionary interface to maintain compliance
with the existing implementation. While people should not be relying on
these constants explicitly, they do flow through the system in ways that
might make their signatures somewhat public.
@keelerm84 keelerm84 requested a review from a team as a code owner October 30, 2024 15:15
@keelerm84 keelerm84 merged commit 06f11f0 into main Oct 30, 2024
6 checks passed
@keelerm84 keelerm84 deleted the mk/sdk-824/process.warmup branch October 30, 2024 15:54
keelerm84 pushed a commit that referenced this pull request Oct 30, 2024
🤖 I have created a release *beep* *boop*
---


##
[8.8.1](8.8.0...8.8.1)
(2024-10-30)


### Bug Fixes

* Modify data kind constants to avoid Process.warmup rehash
([#301](#301))
([06f11f0](06f11f0))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@mperham
Copy link

mperham commented Oct 30, 2024

Is this a bug in Ruby?

@keelerm84
Copy link
Member Author

I am wondering the same thing. I was planning on writing out a standalone reproduction case and then raising the issue.

@mperham
Copy link

mperham commented Oct 30, 2024

I would raise the issue immediately and then work on a repro. The core team may be able to figure out the issue from the behavior without any additional work from you, save yourself the trouble.

@keelerm84
Copy link
Member Author

Posted the issue at https://bugs.ruby-lang.org/issues/20853

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.

3 participants