This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Set gRPC deadlines to all remote index requests
ClosedPublic

Authored by kbobyrev on Jun 30 2020, 12:15 AM.

Diff Detail

Event Timeline

kbobyrev created this revision.Jun 30 2020, 12:15 AM
sammccall accepted this revision.Jun 30 2020, 1:22 AM
sammccall added inline comments.
clang-tools-extra/clangd/index/remote/Client.cpp
63

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. https://github.com/grpc/grpc/blob/master/doc/wait-for-ready.md)

95

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)

This revision is now accepted and ready to land.Jun 30 2020, 1:22 AM
kbobyrev updated this revision to Diff 274744.Jul 1 2020, 3:40 AM
kbobyrev marked 2 inline comments as done.

Address review comments.

This revision was automatically updated to reflect the committed changes.