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/Client.cpp:63
+      std::shared_ptr<grpc::Channel> Channel,
+      std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(500))
+      : Stub(remote::SymbolIndex::NewStub(Channel)),
I fear this may be too short - experience is going to be bad if it's really 
slow, but cutting off requests isn't going to be great.
I'd suggest bumping this up to at least a second... we should be sure we're 
logging timeouts too.

(If the connection is actually down, we shouldn't end up waiting for the 
deadline anyway in general: fail-fast is the default for grpc.

Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:95
+  // Each request will be terminated if it takes too long.
+  const std::chrono::milliseconds DeadlineWaitingTime;
nit: I'd suggest generally avoiding const on non-pointer members, it messes 
with move semantics.

(In this case the object is always wrapped in unique_ptr before being exposed 
publicly, so it's actually fine. But have seen enough cases where it's 
initially fine and later not that I personally prefer to just always avoid it)

  rG LLVM Github Monorepo


cfe-commits mailing list

Reply via email to