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. https://github.com/grpc/grpc/blob/master/doc/wait-for-ready.md) ================ 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) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82844/new/ https://reviews.llvm.org/D82844 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
