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.

Sign up for GitHub

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

Jump to bottom

Use concurrentDictionary for merged options due to thread issue expos… #1979

Merged
merged 2 commits into from
Dec 1, 2022
Merged

Use concurrentDictionary for merged options due to thread issue expos… #1979

merged 2 commits into from
Dec 1, 2022

Conversation

jennyf19
Copy link
Collaborator

@jennyf19 jennyf19 commented Nov 27, 2022 •
edited by jmprieur
Loading

…ed in .NET 7

Fixes #1957

jmprieur
jmprieur approved these changes Nov 28, 2022
View reviewed changes
Copy link
Collaborator

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @jennyf19

@jmprieur
Copy link
Collaborator

tested with 100,000 concurrent calls

@petedishman petedishman mentioned this pull request Nov 29, 2022
Closed
@petedishman
Copy link

Hey @jmprieur @jennyf19
Should this have been merged for the 1.25.9 release? #1981 got merged adding this change to the changelog, but the change itself hasn't been merged so isn't in 1.25.9 from what I can see?

I commented on #1957 about this but that's closed now so don't know if it would get missed.

Thanks

@jennyf19
Copy link
Collaborator Author

jennyf19 commented Dec 1, 2022

Hey @jmprieur @jennyf19 Should this have been merged for the 1.25.9 release? #1981 got merged adding this change to the changelog, but the change itself hasn't been merged so isn't in 1.25.9 from what I can see?

I commented on #1957 about this but that's closed now so don't know if it would get missed.

Thanks

🤦

@jennyf19 jennyf19 merged commit 37b0d33 into master Dec 1, 2022
jmprieur added a commit that referenced this pull request Dec 2, 2022
@jmprieur
Fix for #1957
41bcee6 
Replace IOptionsMonitor<MergedOptions> by
IMergedOptionsStore.

See also #1979 for master (v1.0)
(this PR is for rel/v2.0)
@jmprieur jmprieur mentioned this pull request Dec 2, 2022
Merged
jmprieur added a commit that referenced this pull request Dec 2, 2022
@jmprieur
Fix for #1957 (#1988)
0ea1c87 
Replace IOptionsMonitor<MergedOptions> by
IMergedOptionsStore.

See also #1979 for master (v1.0)
(this PR is for rel/v2.0)
@jmprieur jmprieur deleted the jennyf/fixMergeOpts branch December 8, 2022 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmprieur jmprieur jmprieur approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@jennyf19 @jmprieur @petedishman