Skip to content

Commit ed07932

Browse files
xavierdfacebook-github-bot
authored andcommitted
fsck: remove overlay for directories that aren't on disk
Summary: During startup, EdenFS loads inodes for all the directories present in the overlay, with the assumption that the state of the overlay represents the state of the filesystem. Today, this is mostly true as the working copy only grows and thus the only way for the working copy and overlay to shrink is to update past commits that delete a directory. In the future however, EdenFS will gain the ability to shrink its working copy, either manually, or automatically. When this happens, the overlay will still contain data for directories that are no longer in the working copy, thus invalidating the assumption that working copy and overlay stay in sync. To fix this, we simply need to teach the fsck code to remove overlay directories when a directory isn't present on disk. Reviewed By: kmancini Differential Revision: D41007167 fbshipit-source-id: 2a6d93417995b9bb8f96c757d47b444c17ab82f8
1 parent e1a1982 commit ed07932

2 files changed

Lines changed: 39 additions & 1 deletion

File tree

eden/fs/inodes/sqlitecatalog/WindowsFsck.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,14 @@ std::optional<InodeNumber> fixup(
491491
state.desiredHash,
492492
insensitiveOverlayDir);
493493
} else {
494-
return InodeNumber(*state.overlayEntry->inodeNumber());
494+
auto inodeNumber = InodeNumber(*state.overlayEntry->inodeNumber());
495+
if (!state.onDisk && state.overlayDtype == dtype_t::Dir) {
496+
auto overlayDir = inodeCatalog.loadAndRemoveOverlayDir(inodeNumber);
497+
if (overlayDir) {
498+
XLOGF(DBG9, "Removed overlay directory for: {}", path);
499+
}
500+
}
501+
return inodeNumber;
495502
}
496503
} else {
497504
if (state.inOverlay) {

eden/integration/windows_fsck_test.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import subprocess
1010
import sys
1111
import unittest
12+
from pathlib import Path
1213
from typing import Dict, List, Optional, Set, Tuple, Union
1314

1415
from facebook.eden.ttypes import (
@@ -247,6 +248,36 @@ def test_fsck_rename_with_different_case_and_modify_while_stopped(self) -> None:
247248
self._update_clean()
248249
self.assertEqual(self._eden_status(), {})
249250

251+
def test_loaded_inodes_not_loaded_on_restart(self) -> None:
252+
"""Verifies that a loaded inode not present on disk doesn't get loaded
253+
with a positive refcount on restart.
254+
"""
255+
256+
def get_all_loaded_under(path: str) -> List[Tuple[Path, int]]:
257+
with self.eden.get_thrift_client_legacy() as client:
258+
all_loaded = client.debugInodeStatus(
259+
self.mount_path_bytes,
260+
path.encode(),
261+
0,
262+
sync=SyncBehavior(),
263+
)
264+
265+
ret: List[Tuple[Path, int]] = []
266+
for loaded in all_loaded:
267+
ret += [(Path(loaded.path.decode()), loaded.refcount)]
268+
269+
return ret
270+
271+
# This relies on debugInodeStatus to load the inode for the directory.
272+
loaded = get_all_loaded_under("subdir/bdir")
273+
self.assertEqual(loaded, [(Path("subdir/bdir"), 0)])
274+
275+
self.eden.shutdown()
276+
self.eden.start()
277+
278+
loaded = get_all_loaded_under("subdir/bdir")
279+
self.assertEqual(loaded, [(Path("subdir/bdir"), 0)])
280+
250281

251282
MATERIALIZED = True
252283
UNMATERIALIZED = False

0 commit comments

Comments
 (0)