Use shared pointer to help with high memory utilization #8498
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iocore/net/SSLSessionCache.cc
Outdated
|
|
||
| // Free the shared pointer we're holding right now to make sure the only | ||
| // reference is inside the "SSLOriginSession" object we just created | ||
| shared_sess.reset(); |
Member
There was a problem hiding this comment.
I don't understand this. Isn't is a local variable that will go out of scope at the end of the method?
Contributor
Author
There was a problem hiding this comment.
Left over code from some testing I was doing, just making sure everything is released where it's supposed so I can make sure the ref-count is correct.
duke8253
force-pushed
the
master-orig_sess_share
branch
2 times, most recently
from
23b53d5 to
f8720f6
Compare
duke8253
force-pushed
the
master-orig_sess_share
branch
from
baa3cd1 to
42ebd81
Compare
SolidWallOfCode
approved these changes
Nov 9, 2021
Member
|
This doesn't compile with BoringSSL 😢 Also, the milestone is set to 10-Dev but it is merged to master. I think it should have been merged to 10-Dev branch if I understand the rule correctly. |
zwoop
pushed a commit
that referenced
this pull request
Jan 25, 2022
(cherry picked from commit 664e80e)
Contributor
|
Cherry-picked to v9.2.x |
moonchen
pushed a commit
to moonchen/trafficserver
that referenced
this pull request
Mar 17, 2022
* asf/9.2.x: Updated ChangeLog Add SSLSessionDup for older OpenSSL and BoringSSL (apache#8578) use shared pointer to help with high memory utilization (apache#8498) Commenting TSHttpTxnCacheLookupStatusGet need_to_revalidate (apache#8621) check size of session, and free sessions the ATS way (apache#8330) free sessions when timeout (apache#8356) Fix 32bit build failure on Odroid Xu-4 (apache#8626) TSHttpTxnCacheLookupStatusGet: call need_to_revalidate (apache#8617) SNIConfig (tunnel_route): Change the way we extract matched subgroups from the server name. (apache#8589) fix for collapsed forwarding ink_abort for CacheHitFresh fail (apache#8613) Do not turn off cache for internal requests (apache#8266) Rate Limit Plugin: Re-enable VConnection when SNI is empty (apache#8625) Removes hard dependency on having perl installed (apache#8611)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We've been seeing some high memory utilization after turning on origin session cache in production. After some investigation, I find that the cause is due connections (anywhere from 20~50) could be using the same session at the same time. And since the sessions are stored in a way such that when a new connections requests for a session, it allocates a new one for that connection (to prevent double freeing when connection closes, and avoid using OpenSSL's internal ref-counter to have less ambiguity), it uses a lot more memory than what I expected.
Some calculations here:
Session objects for origin sessions are limited to 4KB each, default cache pool size is 10240, the 50x multiplication factor increase the size from 40MB to 2GB. Not too big of a problem on hosts with lots of memory, but for some smaller ones it is bad news.
The changes in this PR uses shared pointer to share sessions among connections, this will help with high memory utilization. It also eliminates
d2i_SSL_SESSION/i2d_SSL_SESSIONconversions all together, so there's a slight performance improvement as well.