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

Clean routine #530

Merged
merged 19 commits into from
Aug 26, 2020
Merged

Clean routine #530

merged 19 commits into from
Aug 26, 2020

Conversation

vhvb1989
Copy link
Member

fixes: #504

This PR:

  • Separates CurlSession from CurlConnectionPool and CurlConnection.
  • Introduces a clean thread function which gets started as soon as a connection is moved to the connection pool.
    • it runs every 5 minutes and remove any connection expired (on connection pool for more than 1 min)
    • if the clean thread removes all connections, it stop the cleaning until the next time a connection is moved to the connection pool again. This way it is not just running forever when it is not needed.

@Jinming-Hu
Copy link
Member

@vhvb1989 can you merge this PR ASAP? we're going to release on Aug 27 (Shanghai local time).

sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp Outdated Show resolved Hide resolved
sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp Outdated Show resolved Hide resolved
sdk/core/azure-core/test/ut/transport_adapter.cpp Outdated Show resolved Hide resolved
sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp Outdated Show resolved Hide resolved
sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp Outdated Show resolved Hide resolved
sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp Outdated Show resolved Hide resolved
sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp Outdated Show resolved Hide resolved
sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp Outdated Show resolved Hide resolved
sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp Outdated Show resolved Hide resolved
@vhvb1989 vhvb1989 requested a review from a team as a code owner August 26, 2020 22:25
void CurlConnectionPool::CleanUp()
{
std::thread backgroundCleanerThread([]() {
for (;;)
Copy link
Member

Choose a reason for hiding this comment

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

This is an infinite loop. How does it stop when the program exits?

Copy link
Member

Choose a reason for hiding this comment

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

This might crash the whole program

Copy link
Member Author

Choose a reason for hiding this comment

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

it gets stop as soon as there are no connections. If the clean routine removes all connections, the function returns.

if (CurlConnectionPool::s_connectionCounter == 0)
        {
          // stop the cleaner since there are no connections
          CurlConnectionPool::s_isCleanConnectionsRunning = false;
          return;
        }

this way, is the cleaner removes all connections, it would wait and if in the next check there are no connections it returns and thread finishes

Copy link
Member

Choose a reason for hiding this comment

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

but still, you didn't join the thread

@vhvb1989 vhvb1989 merged commit 1f0da6f into Azure:master Aug 26, 2020
@vhvb1989 vhvb1989 deleted the clean-routine branch August 26, 2020 23:51
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.

[libcurl connection pool] Add clean up routine and timmer
4 participants