Skip to content

[6.0] Remove IsAvailable check on CommitAsync. #44610

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

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

adityamandaleeka
Copy link
Member

@adityamandaleeka adityamandaleeka commented Oct 18, 2022

Backport #43059 to 6.0

Remove IsAvailable check on CommitAsync.

This removes an unnecessary call to IsAvailable in CommitAsync that was added during a large change that added nullability annotations in a bunch of places.

Fixes #42009

Customer Impact

In addition to being unnecessary, the IsAvailable call can cause a significant performance hit to the application since it will try to load the session each time on CommitAsync whether or not it needs to.

We've had multiple folks run into this issue and report it. For instance, see #43059 (comment) where someone was using a setup recommended in our official docs for distributed caching and observed a massive CPU usage hit on their servers caused by this issue.

Regression?

  • Yes
  • No

The issue was introduced in 6.0 (#26098)

Risk

  • High
  • Medium
  • Low

Removes a superfluous check.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@ghost ghost added the area-runtime label Oct 18, 2022
@ghost ghost added this to the 6.0.x milestone Oct 18, 2022
@ghost
Copy link

ghost commented Oct 18, 2022

Hi @adityamandaleeka. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@adityamandaleeka
Copy link
Member Author

@adityamandaleeka adityamandaleeka added the Servicing-consider Shiproom approval is required for the issue label Oct 18, 2022
@ghost
Copy link

ghost commented Oct 18, 2022

Hi @adityamandaleeka. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@adityamandaleeka adityamandaleeka added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Oct 18, 2022
@adityamandaleeka
Copy link
Member Author

Tactics approved.

@adityamandaleeka
Copy link
Member Author

@wtgodbe This is ready for the next servicing window.

@dougbu dougbu merged commit 9369e71 into dotnet:release/6.0 Nov 2, 2022
@dougbu dougbu modified the milestones: 6.0.x, 6.0.12 Jan 20, 2023
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants