Skip to content

Comments

fix: Avoid connection hang on Linux#1247

Merged
mergify[bot] merged 1 commit intomasterfrom
paulliu/fix-connection-hang
Dec 14, 2020
Merged

fix: Avoid connection hang on Linux#1247
mergify[bot] merged 1 commit intomasterfrom
paulliu/fix-connection-hang

Conversation

@ninegua
Copy link
Member

@ninegua ninegua commented Dec 13, 2020

When deploying cancan on local Linux machine, dfx bootstrap server will hang during asset upload.

The problem appears to be that TCP connection to the replica's HTTP port was forever stuck in ESTAB state (according to ss -iepn | grep tcp). A similar problem was fixed in reqwest by setting keepalive. However it does not seem to work for actix-web.

I hacked through a set of libraries to set keepalive to the socket, but in the end it still hang even though I could confirm keepalive was set by using ss.

The fix is to limit to only 1 TCP connection per worker to connect to the replica port for now.

@ninegua ninegua requested review from hansl and p-shahi December 13, 2020 11:09
@hansl
Copy link
Contributor

hansl commented Dec 13, 2020

That means all connections from dfx, since they always go through the proxy, would be synchronous, right? This might have unintended consequences. Let me check a bit more about this.

Also, I'm open to having a different HTTP library than reqwest if you found something that could work better.

@ninegua
Copy link
Member Author

ninegua commented Dec 14, 2020

That means all connections from dfx, since they always go through the proxy, would be synchronous, right?

There are by default 4 workers in actix-web/actix-http-client, so it may not be a problem.

Of course the fix here is not ideal, but until we get to the bottom of this issue, we don't have much of a choice to unlock Linux developers.

Copy link
Contributor

@hansl hansl left a comment

Choose a reason for hiding this comment

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

LGTM with the premise that replica is not parallelized right now anyway.

@mergify mergify bot merged commit f21ebd8 into master Dec 14, 2020
@mergify mergify bot deleted the paulliu/fix-connection-hang branch December 14, 2020 23:34
mergify bot pushed a commit that referenced this pull request Aug 16, 2021
#1784)

This is essentially the same fix as in #1247.

We are still seeing this bug https://forum.dfinity.org/t/multiple-calls-queries-to-canister-blocks-them-all-from-sending-back-the-response/6406.

I believe by default there are 4 worker threads so concurrent connections are still possible. Unless we have strong evidence that this kills performance, I think it is better to unblock our users before we find a permanent solution.
dfinity-bot added a commit that referenced this pull request May 20, 2022
## Changelog for advisory-db:
Branch: main
Commits: [rustsec/advisory-db@a47cd630...b4d87867](rustsec/advisory-db@a47cd63...b4d8786)

* [`e1e8e92e`](rustsec/advisory-db@e1e8e92) Add advisory for openssl CVE-2022-1473 ([RustSec/advisory-db⁠#1245](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1245))
* [`4e24c897`](rustsec/advisory-db@4e24c89) Assigned RUSTSEC-2022-0025 to openssl-src ([RustSec/advisory-db⁠#1246](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1246))
* [`999edf88`](rustsec/advisory-db@999edf8) Add advisory for openssl CVE-2022-1434 ([RustSec/advisory-db⁠#1244](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1244))
* [`52b29cd7`](rustsec/advisory-db@52b29cd) Assigned RUSTSEC-2022-0026 to openssl-src ([RustSec/advisory-db⁠#1247](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1247))
* [`c9177664`](rustsec/advisory-db@c917766) Add advisory for openssl CVE-2022-1343 ([RustSec/advisory-db⁠#1243](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1243))
* [`bdc5813f`](rustsec/advisory-db@bdc5813) Assigned RUSTSEC-2022-0027 to openssl-src ([RustSec/advisory-db⁠#1248](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1248))
* [`0abe7433`](rustsec/advisory-db@0abe743) Fix category of RUSTSEC-2022-0025 ([RustSec/advisory-db⁠#1249](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1249))
* [`b4d87867`](rustsec/advisory-db@b4d8786) fix hyper patched version number ([RustSec/advisory-db⁠#1250](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1250))
mergify bot pushed a commit that referenced this pull request May 20, 2022
## Changelog for advisory-db:
Branch: main
Commits: [rustsec/advisory-db@a47cd630...b4d87867](rustsec/advisory-db@a47cd63...b4d8786)

* [`e1e8e92e`](rustsec/advisory-db@e1e8e92) Add advisory for openssl CVE-2022-1473 ([RustSec/advisory-db⁠#1245](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1245))
* [`4e24c897`](rustsec/advisory-db@4e24c89) Assigned RUSTSEC-2022-0025 to openssl-src ([RustSec/advisory-db⁠#1246](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1246))
* [`999edf88`](rustsec/advisory-db@999edf8) Add advisory for openssl CVE-2022-1434 ([RustSec/advisory-db⁠#1244](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1244))
* [`52b29cd7`](rustsec/advisory-db@52b29cd) Assigned RUSTSEC-2022-0026 to openssl-src ([RustSec/advisory-db⁠#1247](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1247))
* [`c9177664`](rustsec/advisory-db@c917766) Add advisory for openssl CVE-2022-1343 ([RustSec/advisory-db⁠#1243](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1243))
* [`bdc5813f`](rustsec/advisory-db@bdc5813) Assigned RUSTSEC-2022-0027 to openssl-src ([RustSec/advisory-db⁠#1248](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1248))
* [`0abe7433`](rustsec/advisory-db@0abe743) Fix category of RUSTSEC-2022-0025 ([RustSec/advisory-db⁠#1249](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1249))
* [`b4d87867`](rustsec/advisory-db@b4d8786) fix hyper patched version number ([RustSec/advisory-db⁠#1250](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/1250))
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.

2 participants