ssl: Avoid partial connection trees #6270
Conversation
CT Test ResultsNo tests were run for this PR. This is either because the build failed, or the PR is based on a branch without GH actions tests configured. Results for commit 3456747 To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
|
Thanks @IngelaAndin ! I can test this out on Monday and let you know if it resolves the issue we were seeing. I am also curious if this new process that starts the connection tree will add any noticeable overhead. I will report back soon. |
IngelaAndin
force-pushed
the
ingela/ssl/dy-sup-leak/GH-6244/OTP-18233
branch
from
bc90049
to
daf3c21
Compare
IngelaAndin
added
the
testing
IngelaAndin
force-pushed
the
ingela/ssl/dy-sup-leak/GH-6244/OTP-18233
branch
from
daf3c21
to
8abcbdd
Compare
|
@sneako How is the testing going? |
|
I was waiting for the requested changes to be addressed so that I could be sure to test the final version. If you think this is ready to go I can start testing this afternoon! |
IngelaAndin
force-pushed
the
ingela/ssl/dy-sup-leak/GH-6244/OTP-18233
branch
from
8abcbdd
to
1fdf101
Compare
|
@sneako well the changes were mainly cosmetic, but I made a new version now. |
If the "User" process, the process starting the TLS connection, gets killed in the middle of spawning the dynamic connection tree make sure we do not leave any processes behind. Close erlang#6244
IngelaAndin
force-pushed
the
ingela/ssl/dy-sup-leak/GH-6244/OTP-18233
branch
2 times, most recently
from
e882332
to
72bafaf
Compare
|
@sneako I added a test case cuddle, but I would not worry about it for the purpose of your testing, the test case is sensitive to timing and testing timeout functionality is tricky, I do not think this is actualy related to my change. |
|
Sounds good! I am going to deploy the test shortly. Will have some results for you by the end of the day. |
|
This patch has been running for almost 4 hours now on a subset of our prod nodes and we have not seen a single instance of a As far as I can tell, this PR seems to have completely resolved the reported issue, and to be extra sure I will let this test continue running for the rest of the day, thanks @IngelaAndin ! |
Wait "long enough" in ssl_api_SUITE. Cosmetic changes to ssl_api_SUITE.
IngelaAndin
force-pushed
the
ingela/ssl/dy-sup-leak/GH-6244/OTP-18233
branch
from
72bafaf
to
3456747
Compare
|
I ended up leaving this test running overnight, almost 24 hours now, and things still look great 😀 |
If the "User" process, the process starting the TLS connection, gets
killed in the middle of spawning the dynamic connection tree make sure
we do not leave any processes behind.
Close #6244