This is an archive of the discontinued LLVM Phabricator instance.

[clangd][remote] Add flag to set idletimeout
ClosedPublic

Authored by kadircet on Feb 26 2021, 1:08 AM.

Details

Summary

By default gRPC has no idletimeout and some firewalls might drop idle
connections after a certain period. This results in idle clients
shouting into void until server resets the connection.

Diff Detail

Event Timeline

kadircet created this revision.Feb 26 2021, 1:08 AM
kadircet requested review of this revision.Feb 26 2021, 1:08 AM
sammccall accepted this revision.Feb 26 2021, 1:26 AM
sammccall added inline comments.
clang-tools-extra/clangd/index/remote/server/Server.cpp
84

10 seems much too low, maybe set this to 5 min or so?
Going idle for a minute or so seems common, and the only real reason we have to time connections out is to beat the gcp firewall. So I don't see the need to be really aggressive here.

This revision is now accepted and ready to land.Feb 26 2021, 1:26 AM
kadircet updated this revision to Diff 326632.Feb 26 2021, 2:07 AM
kadircet marked an inline comment as done.

Change 10 seconds to 600 seconds.

clang-tools-extra/clangd/index/remote/server/Server.cpp
84

oops this was meant to be 10 minutes, not seconds :D

sammccall accepted this revision.Feb 26 2021, 4:21 AM
sammccall added inline comments.
clang-tools-extra/clangd/index/remote/server/Server.cpp
84

Haha, makes more sense :-)

Setting this to exactly the gcp timeout seems tempting fate with hard to debug races. Maybe 8 min?

kadircet updated this revision to Diff 326667.Feb 26 2021, 5:52 AM
kadircet marked an inline comment as done.
  • change default from 10 to 8 minutes
This revision was landed with ongoing or failed builds.Feb 26 2021, 6:05 AM
This revision was automatically updated to reflect the committed changes.
kadircet added inline comments.Feb 26 2021, 6:07 AM
clang-tools-extra/clangd/index/remote/server/Server.cpp
84

fair point, done!