[PATCH] D97536: [clangd][remote] Add flag to set idletimeout
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:84
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(10),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
sammccall wrote:
> kadircet wrote:
> > sammccall wrote:
> > > 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.
> > oops this was meant to be 10 minutes, not seconds :D
> Haha, makes more sense :-)
>
> Setting this to exactly the gcp timeout seems tempting fate with hard to
> debug races. Maybe 8 min?
fair point, done!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97536/new/
https://reviews.llvm.org/D97536
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D97536: [clangd][remote] Add flag to set idletimeout
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a5dfb7db23e: [clangd][remote] Add flag to set idletimeout
(authored by kadircet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97536/new/
https://reviews.llvm.org/D97536
Files:
clang-tools-extra/clangd/index/remote/server/Server.cpp
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -80,6 +80,11 @@
"server-address", llvm::cl::init("0.0.0.0:50051"),
llvm::cl::desc("Address of the invoked server. Defaults to
0.0.0.0:50051"));
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(8 * 60),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
+ "the connection, in seconds. Defaults to 480."));
+
static Key CurrentRequest;
class RemoteIndexServer final : public v1::SymbolIndex::Service {
@@ -311,6 +316,8 @@
grpc::ServerBuilder Builder;
Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());
+ Builder.AddChannelArgument(GRPC_ARG_MAX_CONNECTION_IDLE_MS,
+ IdleTimeoutSeconds * 1000);
Builder.RegisterService(&Service);
std::unique_ptr Server(Builder.BuildAndStart());
log("Server listening on {0}", ServerAddress);
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -80,6 +80,11 @@
"server-address", llvm::cl::init("0.0.0.0:50051"),
llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(8 * 60),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
+ "the connection, in seconds. Defaults to 480."));
+
static Key CurrentRequest;
class RemoteIndexServer final : public v1::SymbolIndex::Service {
@@ -311,6 +316,8 @@
grpc::ServerBuilder Builder;
Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());
+ Builder.AddChannelArgument(GRPC_ARG_MAX_CONNECTION_IDLE_MS,
+ IdleTimeoutSeconds * 1000);
Builder.RegisterService(&Service);
std::unique_ptr Server(Builder.BuildAndStart());
log("Server listening on {0}", ServerAddress);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D97536: [clangd][remote] Add flag to set idletimeout
kadircet updated this revision to Diff 326667.
kadircet marked an inline comment as done.
kadircet added a comment.
- change default from 10 to 8 minutes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97536/new/
https://reviews.llvm.org/D97536
Files:
clang-tools-extra/clangd/index/remote/server/Server.cpp
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -80,6 +80,11 @@
"server-address", llvm::cl::init("0.0.0.0:50051"),
llvm::cl::desc("Address of the invoked server. Defaults to
0.0.0.0:50051"));
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(8 * 60),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
+ "the connection, in seconds. Defaults to 480."));
+
static Key CurrentRequest;
class RemoteIndexServer final : public v1::SymbolIndex::Service {
@@ -311,6 +316,8 @@
grpc::ServerBuilder Builder;
Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());
+ Builder.AddChannelArgument(GRPC_ARG_MAX_CONNECTION_IDLE_MS,
+ IdleTimeoutSeconds * 1000);
Builder.RegisterService(&Service);
std::unique_ptr Server(Builder.BuildAndStart());
log("Server listening on {0}", ServerAddress);
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -80,6 +80,11 @@
"server-address", llvm::cl::init("0.0.0.0:50051"),
llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(8 * 60),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
+ "the connection, in seconds. Defaults to 480."));
+
static Key CurrentRequest;
class RemoteIndexServer final : public v1::SymbolIndex::Service {
@@ -311,6 +316,8 @@
grpc::ServerBuilder Builder;
Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());
+ Builder.AddChannelArgument(GRPC_ARG_MAX_CONNECTION_IDLE_MS,
+ IdleTimeoutSeconds * 1000);
Builder.RegisterService(&Service);
std::unique_ptr Server(Builder.BuildAndStart());
log("Server listening on {0}", ServerAddress);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D97536: [clangd][remote] Add flag to set idletimeout
sammccall accepted this revision.
sammccall added inline comments.
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:84
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(10),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
kadircet wrote:
> sammccall wrote:
> > 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.
> oops this was meant to be 10 minutes, not seconds :D
Haha, makes more sense :-)
Setting this to exactly the gcp timeout seems tempting fate with hard to debug
races. Maybe 8 min?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97536/new/
https://reviews.llvm.org/D97536
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D97536: [clangd][remote] Add flag to set idletimeout
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:84
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(10),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
sammccall wrote:
> 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.
oops this was meant to be 10 minutes, not seconds :D
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97536/new/
https://reviews.llvm.org/D97536
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D97536: [clangd][remote] Add flag to set idletimeout
kadircet updated this revision to Diff 326632.
kadircet marked an inline comment as done.
kadircet added a comment.
Change 10 seconds to 600 seconds.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97536/new/
https://reviews.llvm.org/D97536
Files:
clang-tools-extra/clangd/index/remote/server/Server.cpp
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -80,6 +80,11 @@
"server-address", llvm::cl::init("0.0.0.0:50051"),
llvm::cl::desc("Address of the invoked server. Defaults to
0.0.0.0:50051"));
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(10 * 60),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
+ "the connection, in seconds. Defaults to 600."));
+
static Key CurrentRequest;
class RemoteIndexServer final : public v1::SymbolIndex::Service {
@@ -311,6 +316,8 @@
grpc::ServerBuilder Builder;
Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());
+ Builder.AddChannelArgument(GRPC_ARG_MAX_CONNECTION_IDLE_MS,
+ IdleTimeoutSeconds * 1000);
Builder.RegisterService(&Service);
std::unique_ptr Server(Builder.BuildAndStart());
log("Server listening on {0}", ServerAddress);
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -80,6 +80,11 @@
"server-address", llvm::cl::init("0.0.0.0:50051"),
llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(10 * 60),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
+ "the connection, in seconds. Defaults to 600."));
+
static Key CurrentRequest;
class RemoteIndexServer final : public v1::SymbolIndex::Service {
@@ -311,6 +316,8 @@
grpc::ServerBuilder Builder;
Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());
+ Builder.AddChannelArgument(GRPC_ARG_MAX_CONNECTION_IDLE_MS,
+ IdleTimeoutSeconds * 1000);
Builder.RegisterService(&Service);
std::unique_ptr Server(Builder.BuildAndStart());
log("Server listening on {0}", ServerAddress);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D97536: [clangd][remote] Add flag to set idletimeout
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:84
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(10),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97536/new/
https://reviews.llvm.org/D97536
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D97536: [clangd][remote] Add flag to set idletimeout
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D97536
Files:
clang-tools-extra/clangd/index/remote/server/Server.cpp
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -80,6 +80,11 @@
"server-address", llvm::cl::init("0.0.0.0:50051"),
llvm::cl::desc("Address of the invoked server. Defaults to
0.0.0.0:50051"));
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(10),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
+ "the connection, in seconds. Defaults to 10."));
+
static Key CurrentRequest;
class RemoteIndexServer final : public v1::SymbolIndex::Service {
@@ -311,6 +316,8 @@
grpc::ServerBuilder Builder;
Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());
+ Builder.AddChannelArgument(GRPC_ARG_MAX_CONNECTION_IDLE_MS,
+ IdleTimeoutSeconds * 1000);
Builder.RegisterService(&Service);
std::unique_ptr Server(Builder.BuildAndStart());
log("Server listening on {0}", ServerAddress);
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -80,6 +80,11 @@
"server-address", llvm::cl::init("0.0.0.0:50051"),
llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
+llvm::cl::opt IdleTimeoutSeconds(
+"idle-timeout", llvm::cl::init(10),
+llvm::cl::desc("Maximum time a channel may stay idle until server closes "
+ "the connection, in seconds. Defaults to 10."));
+
static Key CurrentRequest;
class RemoteIndexServer final : public v1::SymbolIndex::Service {
@@ -311,6 +316,8 @@
grpc::ServerBuilder Builder;
Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());
+ Builder.AddChannelArgument(GRPC_ARG_MAX_CONNECTION_IDLE_MS,
+ IdleTimeoutSeconds * 1000);
Builder.RegisterService(&Service);
std::unique_ptr Server(Builder.BuildAndStart());
log("Server listening on {0}", ServerAddress);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
