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

remove httpcontext from rolename initializer #1340

Merged
merged 7 commits into from
Dec 6, 2019

Conversation

TimothyMothra
Copy link
Member

No description provided.

@TimothyMothra TimothyMothra added this to the 2.12 milestone Dec 5, 2019
@TimothyMothra
Copy link
Member Author

@Dmitry-Matveev, we need to re-test performance after this merges


set
{
if (value != roleName)
Copy link
Member

Choose a reason for hiding this comment

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

why not using Interlocked.CompareExchange for comparison and exchange?

Copy link
Member Author

Choose a reason for hiding this comment

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

Simple answer is, i didn't know the correct way to use CompareExchange.
Now that I have better test coverage, i'll take another look at this. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

If you know something I don't, please share. :)

From what I'm reading, CompareExchange will always replace the ref variable with an incoming value.
In my use case, I only need to replace the ref variable when the incoming value does not match.
Specifically, when a VIP SWAP occurs, we need to update the role name.
My understanding is, replacing this with a CompareExchange will always replace the ref variable with the incoming value regardless of if it's new.

I believe CompareExchange doesn't help me in this use case.
But please correct me if i'm wrong.

@TimothyMothra TimothyMothra merged commit 467c661 into develop Dec 6, 2019
@TimothyMothra TimothyMothra deleted the tilee/httpcontext_rolename branch December 6, 2019 01:10
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.

4 participants