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

Set idle socket keepalive time to 4 seconds #11631

Closed
wants to merge 13 commits into from

Conversation

ghoshkaj
Copy link
Contributor

@ghoshkaj ghoshkaj commented Aug 23, 2022

The tinylicious tests infrequently but consistently fail with 502 error code. A possible cause might be because the node (Axios) server socket timeout is 15s, whereas the underlying socket timeout default timeout is 5s. This PR switches to using agentkeepalive package that forces the Node server to understand and close connections idle sockets after 4 seconds so that Node doesn't try to use an already closed socket.

Implementing this fix on both client calls and internal server calls, since both locations can have this problem.

@github-actions github-actions bot added area: server Server related issues (routerlicious) base: main PRs targeted against main branch labels Aug 23, 2022
@@ -20,7 +20,7 @@ import { TinyliciousRunnerFactory } from "./runnerFactory";
// a monolithic process, it still uses sockets to communicate with the historian service. For these call,
// keep the TCP connection open so that they can be reused
// TODO: Set this globally since the Historian use the global Axios default instance. Make this encapsulated.
Axios.defaults.httpAgent = new http.Agent({ keepAlive: true });
Axios.defaults.httpAgent = new http.Agent({ keepAlive: true, timeout: (60 * 1000) * 4 });
Copy link
Member

@curtisman curtisman Aug 23, 2022

Choose a reason for hiding this comment

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

I don't think timeout is related to keepAlive, it is just a general timeout for the socket.
I beileve you will need to use agentkeepalive package, which has an extra option to specify freeSocketTimeout. I don't think we need to specify a number, as it is already default to 4000ms.

@ghoshkaj ghoshkaj changed the title Debug flaky 502 tests Set idle socket keepalive time to 5 seconds Aug 24, 2022
@ghoshkaj ghoshkaj marked this pull request as ready for review August 24, 2022 16:07
@ghoshkaj ghoshkaj requested review from msfluid-bot and a team as code owners August 24, 2022 16:07
@ghoshkaj ghoshkaj changed the title Set idle socket keepalive time to 5 seconds Set idle socket keepalive time to 4 seconds Aug 24, 2022
@ghoshkaj ghoshkaj requested a review from a team as a code owner August 24, 2022 17:08
@github-actions github-actions bot added the area: tests Tests to add, test infrastructure improvements, etc label Aug 24, 2022
@ghoshkaj
Copy link
Contributor Author

Sadly, I don't think this is fixing the error after all. 4/10 runs failed:
Failed runs:

Pipelines - Run 87750 (azure.com)
Pipelines - Run 87771 (azure.com)
Pipelines - Run 87772 (azure.com)
Pipelines - Run 87776 (azure.com)

@ghoshkaj ghoshkaj requested review from a team as code owners August 24, 2022 20:18
@ghoshkaj ghoshkaj requested review from a team as code owners August 24, 2022 20:18
@github-actions github-actions bot added area: build Build related issues area: dds Issues related to distributed data structures area: dds: sharedstring area: website dependencies Pull requests that update a dependency file labels Aug 24, 2022
@github-actions
Copy link
Contributor

🔗 Found some broken links! 💔

Run a link check locally to find them. See
https://github.com/microsoft/FluidFramework/wiki/Checking-for-broken-links-in-the-documentation for more information.

linkcheck output


> fluidframework-docs@0.25.0 linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> npm run linkcheck:fast -- --external


> fluidframework-docs@0.25.0 linkcheck:fast /home/runner/work/FluidFramework/FluidFramework/docs
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt "--external"

Crawling...

http://localhost:1313/css/style.min.f22f6c22fcc37a0d857a17c9c1202a67c28deef90cc26a52b7539b903f623331.css
- (1:1184) url(...) => https://c.s-microsoft.com/static/fonts/segoe-ui/west-european/semilight/latest.eot (HTTP 503)
- (1:1272) url(...) => https://c.s-microsoft.com/static/fonts/segoe-ui/west-european/semilight/latest.eot? (HTTP 503)
- (1:1740) url(...) => https://c.s-microsoft.com/static/fonts/segoe-ui/west-european/semilight/latest.svg (HTTP 503)

http://localhost:1313/docs/apis/core-interfaces/ifluidcodedetailscomparer
- (873:360) 'Array.sort' => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description (HTTP 200 but missing anchor)
- (877:168) 'https://..' => https://github.com/npm/node-semver#usage (HTTP 200 but missing anchor)
- (885:25) 'Array.sort' => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description (HTTP 200 but missing anchor)
- (914:25) 'https://..' => https://github.com/npm/node-semver#usage (HTTP 200 but missing anchor)

http://localhost:1313/docs/apis/core-interfaces/ifluidcodedetailscomparer/
- (873:360) 'Array.sort' => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description (HTTP 200 but missing anchor)
- (877:168) 'https://..' => https://github.com/npm/node-semver#usage (HTTP 200 but missing anchor)
- (885:25) 'Array.sort' => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description (HTTP 200 but missing anchor)
- (914:25) 'https://..' => https://github.com/npm/node-semver#usage (HTTP 200 but missing anchor)


Summary of most serious issues:

http://localhost:1313/css/style.min.f22f6c22fcc37a0d857a17c9c1202a67c28deef90cc26a52b7539b903f623331.css
- (1:1184) url(...) => https://c.s-microsoft.com/static/fonts/segoe-ui/west-european/semilight/latest.eot (HTTP 503)
- (1:1272) url(...) => https://c.s-microsoft.com/static/fonts/segoe-ui/west-european/semilight/latest.eot? (HTTP 503)
- (1:1740) url(...) => https://c.s-microsoft.com/static/fonts/segoe-ui/west-european/semilight/latest.svg (HTTP 503)


Stats:
  121031 links
    1316 destination URLs
       1 URLs ignored
       8 warnings
       3 errors


@ghoshkaj ghoshkaj closed this Aug 24, 2022
@ghoshkaj ghoshkaj deleted the set-keepalive-timout-4s branch August 25, 2022 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues area: dds: sharedstring area: dds Issues related to distributed data structures area: server Server related issues (routerlicious) area: tests Tests to add, test infrastructure improvements, etc area: website base: main PRs targeted against main branch dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants