facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-05 14:28:17 +03:00
Code Issues Projects Releases Wiki Activity

listen to PRE_CONVERT_TO_FULL

Browse Source 
Summary:
Andrew Krieger reported that eden doesn't seem to notice when files are
truncated on windows. And it repros pretty easily:

```
fbclone fbsource tmpfbsource
cd tmpfbsource/arvr/libraries/avatar/Libraries/asset/items/gltf
Clear-Content src/GltfCompactSkinningData.cpp
hg st
# empty
```

Eden is getting a file closed and NOT modified notification. Microsoft has
confirmed that this is unexpected and would fix it.

But in the meantime there is something we can do to work around it. We do get
the notification "pre convert to full". Full files should be materialized in
EdenFS, so we can use this notification to mark the inode as materialized early.

Handing notifications is async, so this should not effect performance.
Benchmarking creating a lot of files, shows that this doesn't seem to make much
of a difference.

We will roll this out with a config, and then roll it back once microsoft's fix
rolls out.

Reviewed By: xavierd

Differential Revision: D42465547

fbshipit-source-id: cc41fa8c7ad7fbf96ae4a9af5b3a7e6f477fc449
This commit is contained in:
Katie Mancini 2023-02-14 16:31:47 -08:00 committed by Facebook GitHub Bot
parent 8f67650ca1
commit cb5244b09f
11 changed files with 147 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
eden/fs/config/EdenConfig.h
View File

@ -570,6 +570,18 @@ class EdenConfig : private ConfigSettingManager {
std::chrono::milliseconds(100),
this};
/**
* Listen to pre convert to full notifications from ProjFS. By the spec these
* should not give us any information that the other notifications all ready
* cover. However, ProjFS currently (Feb 2023) has a bug: we do not receive
* the file closed and modified notification. We can listen to this instead
* to ensure our in memory state reflects file truncations.
*/
ConfigSetting<bool> prjfsListenToPreConvertToFull{
"prjfs:listen-to-pre-convert-to-full",
false,
this};
// [hg]
/**

3
eden/fs/inodes/EdenMount.cpp
View File

@ -2028,7 +2028,8 @@ folly::Future<folly::Unit> EdenMount::channelMount(bool readOnly) {
this->getServerState()->getNotifier());
channel->start(
readOnly,
edenConfig->prjfsUseNegativePathCaching.getValue());
edenConfig->prjfsUseNegativePathCaching.getValue(),
edenConfig->prjfsListenToPreConvertToFull.getValue());
return channel;
})
.thenTry([this, mountPromise](

9
eden/fs/inodes/PrjfsDispatcherImpl.cpp
View File

@ -798,6 +798,15 @@ ImmediateFuture<folly::Unit> PrjfsDispatcherImpl::preDirDelete(
return folly::unit;
}
ImmediateFuture<folly::Unit> PrjfsDispatcherImpl::preFileConvertedToFull(
RelativePath path,
const ObjectFetchContextPtr& context) {
// this is an asynchonous notification, so we have to treat this just like
// all the other write notifications.
return fileNotification(
*mount_, std::move(path), notificationExecutor_, context);
}
ImmediateFuture<folly::Unit>
PrjfsDispatcherImpl::waitForPendingNotifications() {
// Since the executor is a SequencedExecutor, and the fileNotification

4
eden/fs/inodes/PrjfsDispatcherImpl.h
View File

@ -79,6 +79,10 @@ class PrjfsDispatcherImpl : public PrjfsDispatcher {
RelativePath relPath,
const ObjectFetchContextPtr& context) override;
ImmediateFuture<folly::Unit> preFileConvertedToFull(
RelativePath relPath,
const ObjectFetchContextPtr& context) override;
ImmediateFuture<folly::Unit> waitForPendingNotifications() override;
private:

2
eden/fs/inodes/test/CheckoutTest.cpp
View File

@ -1750,7 +1750,7 @@ class FakePrjfsChannel : public PrjfsChannel {
&mount->getStraceLogger(),
mount->getServerState()->getProcessNameCache(),
mount->getCheckoutConfig()->getRepoGuid());
channel->start(false, false);
channel->start(false, false, true);
mount->setTestPrjfsChannel(std::move(channel));
}

35
eden/fs/prjfs/PrjfsChannel.cpp
View File

@ -284,7 +284,8 @@ const std::unordered_map<PRJ_NOTIFICATION, PrjfsTraceCallType>
PrjfsTraceCallType::FILE_HANDLE_CLOSED_FILE_DELETED},
{PRJ_NOTIFICATION_PRE_SET_HARDLINK,
PrjfsTraceCallType::PRE_SET_HARDLINK},
};
{PRJ_NOTIFICATION_FILE_PRE_CONVERT_TO_FULL,
PrjfsTraceCallType::FILE_PRE_CONVERT_TO_FULL}};
} // namespace
HRESULT notification(
@ -932,6 +933,14 @@ std::string preSetHardlinkRenderer(
return fmt::format(FMT_STRING("link({} -> {})"), oldPath, newPath);
}
std::string preConvertToFullRenderer(
RelativePathPiece relPath,
RelativePathPiece /*destPath*/,
bool isDirectory) {
return fmt::format(
FMT_STRING("preConvertToFull({}, isDirectory={})"), relPath, isDirectory);
}
const std::unordered_map<PRJ_NOTIFICATION, NotificationHandlerEntry>
notificationHandlerMap = {
{
@ -982,6 +991,12 @@ const std::unordered_map<PRJ_NOTIFICATION, NotificationHandlerEntry>
preSetHardlinkRenderer,
&PrjfsStats::preSetHardlink},
},
{
PRJ_NOTIFICATION_FILE_PRE_CONVERT_TO_FULL,
{&PrjfsChannelInner::preConvertToFull,
preConvertToFullRenderer,
&PrjfsStats::preConvertToFull},
},
};
} // namespace
@ -1080,6 +1095,14 @@ ImmediateFuture<folly::Unit> PrjfsChannelInner::preSetHardlink(
fmt::format(FMT_STRING("Hardlinks are not supported: {}"), relPath)));
}
ImmediateFuture<folly::Unit> PrjfsChannelInner::preConvertToFull(
RelativePath relpath,
RelativePath /*destPath*/,
bool /*isDirectory*/,
const ObjectFetchContextPtr& context) {
return dispatcher_->preFileConvertedToFull(std::move(relpath), context);
}
HRESULT PrjfsChannelInner::notification(
std::shared_ptr<PrjfsRequestContext> context,
const PRJ_CALLBACK_DATA* callbackData,
@ -1175,7 +1198,10 @@ PrjfsChannel::~PrjfsChannel() {
<< "stop() must be called before destroying the channel";
}
void PrjfsChannel::start(bool readOnly, bool useNegativePathCaching) {
void PrjfsChannel::start(
bool readOnly,
bool useNegativePathCaching,
bool prjfsListenToPreConvertToFull) {
if (readOnly) {
NOT_IMPLEMENTED();
}
@ -1200,6 +1226,11 @@ void PrjfsChannel::start(bool readOnly, bool useNegativePathCaching) {
L""},
};
if (prjfsListenToPreConvertToFull) {
notificationMappings[0].NotificationBitMask |=
PRJ_NOTIFY_FILE_PRE_CONVERT_TO_FULL;
}
auto startOpts = PRJ_STARTVIRTUALIZING_OPTIONS();
startOpts.NotificationMappings = notificationMappings;
startOpts.NotificationMappingsCount =

16
eden/fs/prjfs/PrjfsChannel.h
View File

@ -303,6 +303,15 @@ class PrjfsChannelInner {
bool isDirectory,
const ObjectFetchContextPtr& context);
/**
* Notification sent prior to a file being converted to a full file.
*/
ImmediateFuture<folly::Unit> preConvertToFull(
RelativePath relpath,
RelativePath destPath,
bool isDirectory,
const ObjectFetchContextPtr& context);
ProcessAccessLog& getProcessAccessLog() {
return processAccessLog_;
}
@ -423,7 +432,10 @@ class PrjfsChannel {
virtual ~PrjfsChannel();
void start(bool readOnly, bool useNegativePathCaching);
void start(
bool readOnly,
bool useNegativePathCaching,
bool prjfsListenToPreConvertToFull);
/**
* Wait for all the received notifications to be fully handled.
@ -500,6 +512,8 @@ class PrjfsChannel {
ProcessAccessLog processAccessLog_;
std::shared_ptr<ReloadableConfig> config_;
folly::AtomicReadMostlyMainPtr<PrjfsChannelInner> inner_;
folly::SemiFuture<folly::Unit> innerDeleted_;

7
eden/fs/prjfs/PrjfsDispatcher.h
View File

@ -157,6 +157,13 @@ class PrjfsDispatcher {
RelativePath relPath,
const ObjectFetchContextPtr& context) = 0;
/**
* Notification sent when a file is about to be converted to a full file.
*/
virtual ImmediateFuture<folly::Unit> preFileConvertedToFull(
RelativePath relPath,
const ObjectFetchContextPtr& context) = 0;
/**
* Wait for all received notifications to complete.
*/

1
eden/fs/telemetry/EdenStats.h
View File

@ -248,6 +248,7 @@ struct PrjfsStats : StatsGroup<PrjfsStats> {
Duration preRenamed{"prjfs.preRenamed_us"};
Duration fileHandleClosedFileDeleted{"prjfs.fileHandleClosedFileDeleted_us"};
Duration preSetHardlink{"prjfs.preSetHardlink_us"};
Duration preConvertToFull{"prjfs.preConvertToFull_us"};
Duration openDir{"prjfs.opendir_us"};
Duration readDir{"prjfs.readdir_us"};

1
eden/integration/lib/skip.py
View File

@ -303,6 +303,7 @@ if sys.platform != "win32":
"windows_fsck_test.WindowsFsckTest": True,
"windows_fsck_test.WindowsRebuildOverlayTest": True,
"prjfs_stress.PrjFSStress": True,
"prjfs_stress.PrjfsStressNoListenToFull": True,
"projfs_buffer.PrjFSBuffer": True,
}
)

65
eden/integration/prjfs_stress.py
View File

@ -6,9 +6,10 @@
import os
import stat
import subprocess
from contextlib import contextmanager
from pathlib import Path
from typing import Dict, Generator, Optional, Set, Tuple
from typing import Dict, Generator, List, Optional, Set, Tuple
from eden.fs.cli import util
from facebook.eden.constants import DIS_REQUIRE_MATERIALIZED
@ -22,12 +23,12 @@ from facebook.eden.ttypes import (
from .lib import testcase
@testcase.eden_repo_test
class PrjFSStress(testcase.EdenRepoTest):
class PrjFSStressBase(testcase.EdenRepoTest):
enable_fault_injection: bool = True
def populate_repo(self) -> None:
self.repo.write_file("hello", "hola\n")
self.repo.write_file("adir/file", "foo!\n")
self.repo.commit("Initial commit.")
def edenfs_logging_settings(self) -> Dict[str, str]:
@ -125,6 +126,14 @@ class PrjFSStress(testcase.EdenRepoTest):
)
)
@testcase.eden_repo_test
class PrjFSStress(PrjFSStressBase):
def edenfs_extra_config(self) -> Optional[Dict[str, List[str]]]:
result = super().edenfs_extra_config() or {}
result.setdefault("prjfs", []).append("listen-to-pre-convert-to-full = true")
return result
def test_create_and_remove_file(self) -> None:
with self.run_with_fault():
self.touch("foo")
@ -214,6 +223,7 @@ class PrjFSStress(testcase.EdenRepoTest):
self.assertAllMaterialized(
{
("adir", stat.S_IFDIR),
("bar", stat.S_IFDIR),
("bar/bar", stat.S_IFREG),
("bar/baz", stat.S_IFREG),
@ -249,6 +259,7 @@ class PrjFSStress(testcase.EdenRepoTest):
{
("a/b", stat.S_IFREG),
("a", stat.S_IFDIR),
("adir", stat.S_IFDIR),
("hello", stat.S_IFREG),
},
waitTime=0,
@ -259,6 +270,7 @@ class PrjFSStress(testcase.EdenRepoTest):
{
("a/b", stat.S_IFREG),
("a", stat.S_IFDIR),
("adir", stat.S_IFDIR),
("hello", stat.S_IFREG),
}
)
@ -292,6 +304,7 @@ class PrjFSStress(testcase.EdenRepoTest):
{
("a/b", stat.S_IFDIR),
("a", stat.S_IFDIR),
("adir", stat.S_IFDIR),
("hello", stat.S_IFREG),
("z", stat.S_IFDIR),
("z/x", stat.S_IFREG),
@ -307,6 +320,7 @@ class PrjFSStress(testcase.EdenRepoTest):
("a/b/y", stat.S_IFREG),
("a/b", stat.S_IFDIR),
("a", stat.S_IFDIR),
("adir", stat.S_IFDIR),
("hello", stat.S_IFREG),
}
)
@ -329,6 +343,7 @@ class PrjFSStress(testcase.EdenRepoTest):
self.assertAllMaterialized(
{
("adir", stat.S_IFDIR),
("first", stat.S_IFDIR),
("first/c", stat.S_IFREG),
("first/d", stat.S_IFREG),
@ -338,3 +353,47 @@ class PrjFSStress(testcase.EdenRepoTest):
("hello", stat.S_IFREG),
}
)
def test_truncate(self) -> None:
rel_path = "adir/file"
path = self.mount_path / rel_path
self.assertNotMaterialized(rel_path)
subprocess.run(["powershell.exe", "Clear-Content", str(path)])
# file should be materialized at this point.
self.assertMaterialized(rel_path, stat.S_IFREG)
st = os.lstat(path)
with path.open("rb") as f:
read_back = f.read().decode()
self.assertEqual("", read_back)
self.assertEqual(0, st.st_size)
@testcase.eden_repo_test
class PrjfsStressNoListenToFull(PrjFSStressBase):
def edenfs_extra_config(self) -> Optional[Dict[str, List[str]]]:
result = super().edenfs_extra_config() or {}
result.setdefault("prjfs", []).append("listen-to-pre-convert-to-full = false")
return result
# this test should start failing once msft fixes the bug on their side.
# i.e. once truncation starts to send file closed and modified notifications.
def test_truncate(self) -> None:
rel_path = "adir/file"
path = self.mount_path / rel_path
self.assertNotMaterialized(rel_path)
subprocess.run(["powershell.exe", "Clear-Content", str(path)])
# file should be materialized at this point.
self.assertNotMaterialized(rel_path, stat.S_IFREG)
st = os.lstat(path)
with path.open("rb") as f:
read_back = f.read().decode()
self.assertEqual("", read_back)
self.assertEqual(0, st.st_size)