[PATCH] D55417: [clangd] Change diskbackedstorage to be atomic

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

NIT: I believe we're missing a hyphen in the change description, i.e. it should be `disk-backed`.



================
Comment at: clangd/index/BackgroundIndexStorage.cpp:76
+    // Write to a temporary file first.
+    llvm::SmallString<64> TempPath;
+    int FD;
----------------
Maybe extract this into a helper function?
Something like
```
std::error_code writeAtomically(StringRef OutPath, function_ref<void(raw_ostream&)> Writer) {
 /// create temp file, open output stream.
 Writer(OS):
 /// Move the temp file into OutPath.
}
```

This would keep the `storeShard` function as readable as it is now


================
Comment at: clangd/index/BackgroundIndexStorage.cpp:78
+    int FD;
+    auto EC = llvm::sys::fs::createTemporaryFile("clangd", "index-shard", FD,
+                                                 TempPath);
----------------
Maybe create these temporary files in the same dir? That would ensure the move operations do not require transferring data between physical devices.
If the output file is `/some/path/foo.bar`, we could create files named `/some/path/foo.bar.tmp#####`



Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55417