[PATCH] D98246: [clangd] Add basic monitoring info request for remote index server

Kirill Bobyrev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 15 13:26:53 PDT 2021


kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:308
+                          .count());
+    // FIXME(kirillbobyrev): This is not really "freshness", this is just the
+    // time since last index reload.
----------------
kadircet wrote:
> what's the issue to be fixed here exactly? do we want to display the index build time instead of age? (if so what's the blocker)
The point here is that is the index load freshness but it's not the index load, not the index age itself. The index age is e.g. the time of the last commit being indexed. Hence, we don't actually have this information _right now_ - it should be provided separately (probably in a way of adjacent `index.metadata` file or something similar). E.g. when we download from GitHub release, we do know the time an index was uploaded, that's a better estimate of the index "age" probably. But what the comment in Proto file says is actually more like the commit time, e.g. when did the last change that made it into the index happen.


================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:457
 
-  std::thread HotReloadThread([&Index, &Status, &FS]() {
+  Monitor Monitor(std::chrono::system_clock::now());
+
----------------
kadircet wrote:
> unless we call updateIndex here too, monitoring service won't know about the one being served until the first reload right?
> 
> (another option would be to just not load the index at startup immediately, and handle the initial load in HotReloadThread, but that can be a separate change.)
I think using `Status.getLastModificationTime()` should be correct, right? This is the index we actually loaded (checked above).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98246/new/

https://reviews.llvm.org/D98246



More information about the llvm-commits mailing list