Summary:
As code review follow-up from D8321501 and D8330070, have getFilePath
return InodePath rather than the index to the null terminator.
Also move getFileTmpPath and ensureTmpDirectoryCreated into their own
functions
Reviewed By: strager
Differential Revision: D8363811
fbshipit-source-id: 42718ed4af07fc7a6600564cc7d934f85fe333ed
Summary:
Creating new Overlay entries in tmp/ and then moving to their home is
even faster than creating them in the root of the overlay.
Reviewed By: strager
Differential Revision: D8330070
fbshipit-source-id: 620dde8fb6ccf9bfdc10872f5911d02fea28fdb0
Summary:
Writing to the overlay is excessively expensive on XFS and, to a
lesser extent, Btrfs. Placing temporary files in the root of the
overlay is dramatically faster than placing them next to their
destination.
Reviewed By: simpkins, strager
Differential Revision: D8321501
fbshipit-source-id: f6c9d5dff9d3eb1d393ff6c760518bc65c361aa2
Summary:
To get a read on whether there's anything egregious about the
overlay on various filesystem types and whether we can make any quick
improvements, I wrote up this quick benchmark.
Reviewed By: strager
Differential Revision: D8310783
fbshipit-source-id: 15fa1ea4bcbb7e4896e52b4acf0f8802133dbf3c
Summary:
Per a conversation with simpkins when code reviewing D7882648, this
diff removes the inheritance relationship between TreeInodeState and
DirContents. It doesn't change the binary layout of anything, but
defines DirContents as a typedef of PathMap<DirEntry>.
Reviewed By: strager
Differential Revision: D8232052
fbshipit-source-id: a2166f3ca2ab90fabbded0e48307b8a92a2b0250
Summary:
Several places in edenfs need to represent empty future objects, and were
written before the Future::makeEmpty() method was added. These locations
used Optional<Future> as a workaround.
This updates the code to simply use empty Futures instead of Optional<Future>
now.
Reviewed By: wez
Differential Revision: D8393712
fbshipit-source-id: eeb9e347d0973a4ab602500ee24fba77277d01ea
Summary:
The two FileInode::isSameAs() methods called FileInode::getSha1(), which
returns a folly::Future object, and immediately called `value()` on the
resulting future without waiting for it to complete first.
This bug dates back to D5430128. However, I suspect it was D7888344 that
actually exposed this issue recently. D7888344 updates the
`RocksDbLocalStore` code to perform work in an I/O thread pool. Before this
change the SHA1 futures were likely always immediately ready, so this bug was
not causing problems.
Reviewed By: wez
Differential Revision: D8393671
fbshipit-source-id: ec2116751ddda31a119bfe85eab5612b622f83cf
Summary:
Up until now all of the privhelper APIs have been blocking calls. This
changes the privhelper functions to return Futures, and updates all users of
these APIs to be able to handle the results using Futures.
One benefit of this change is that all existing mount points are remounted in
parallel now during startup, rather than being mounted serially. The old code
performed a blocking `get()` call on the future returned by
`EdenServer::mount()`.
The privhelper calls themselves are still blocking for now--they block until
complete and always return completed Future objects. I will update the
privhelper code in a subsequent diff to actually make it asynchronous.
Reviewed By: bolinfest
Differential Revision: D8053421
fbshipit-source-id: 342d38697f67518f6ca96a37c12dd9812ddb151d
Summary:
Per
35ba669307,
if the return value of DCHECK_NOT_NULL is expected to be unused,
DCHECK should be used instead.
Reviewed By: strager
Differential Revision: D8336319
fbshipit-source-id: 9ea758502baead8941b274dc0ed38ce59b1cc136
Summary:
While I'm in here, borrow the top two bits from mode_t for hasHash_
and hasInodePointer_, making DirEntry fit in four words.
Eventually I want to replace mode_t with dtype_t, but that can't be
done until migration to the InodeMetadataTable is mostly complete. If
I made this change too early, we might lose some of the mode bits
specified when creating a file. If said mode bits resulted in a change
to u+x, the file could look changed relative to source control.
I updated some of the DirEntry documentation while I was at it.
Reviewed By: simpkins
Differential Revision: D7941582
fbshipit-source-id: f62e58f3737c1189ea17cd434b6fef14af359e0a
Summary:
This fixes a bug simpkins pointed out in D6891479 - we weren't
updating mtime and ctime on renames.
Reviewed By: simpkins
Differential Revision: D7937303
fbshipit-source-id: 08fd8f4fe5d99d33e9f7629965d6146330c8f35b
Summary:
Per code review feedback from D6891479, this diff enforces that
metadata writes and reads are done while the corresponding inode's
state lock is held.
Reviewed By: simpkins
Differential Revision: D7884463
fbshipit-source-id: d0e7a95415c280441276452ece7233d4cbf5e942
Summary:
1. Enabled a number of additional C++ compiler warnings in Eden.
2. Fixed warnings-turned-errors that resulted from this change.
Reviewed By: simpkins
Differential Revision: D8132543
fbshipit-source-id: 2290ffaaab55024d582e29201a1bcaa1152e6b3e
Summary:
Like D7867399, split TreeInode's synchronized state into a top-level
class. This is a step towards using the type system to perform
lock-safe metadata updates.
Reviewed By: simpkins
Differential Revision: D7882648
fbshipit-source-id: 27262df8ed9137c8478c68ebf4c4f13878655754
Summary:
Eden will often have a significant number of trees loaded - this saves
8 bytes per entry per loaded TreeInode. It also makes it clear that
once an inode pointer is assigned, the inode number is redundant.
Reviewed By: simpkins
Differential Revision: D7869662
fbshipit-source-id: 21a8266ff5189d3ba9cb614a325cc9d8c3ca305e
Summary:
Shrinking Entry to five words was a bit tricky with some subtle
situations, so I split it into two diffs to verify the tests passed
after each stage.
This diff replaces folly::Optional<Hash> with a boolean and Hash.
Reviewed By: simpkins
Differential Revision: D7869502
fbshipit-source-id: 2df109472d9565e96e8621407f62a63b4f1dbcad
Summary:
FileInode and TreeInode's State classes are complicated enough that
they deserve to be lifted out. In addition, this is necessary for
using the type system to enforce that contents locks are held in
InodeBase's metadata accessors.
Reviewed By: simpkins
Differential Revision: D7867399
fbshipit-source-id: 6ce082149ba02099487e8caed33a7bd8510dfebb
Summary:
Remember the maximum inode number in a file when the Overlay is shut
down cleanly, avoiding a full Overlay scan.
Reviewed By: simpkins
Differential Revision: D7912647
fbshipit-source-id: 8adbafed15259af668a221baa829e1b1f44090d7
Summary:
The Overlay is the natural home for nextInodeNumber_ now that every
directory allocates inode numbers for its children right away. This
also simplifies serializing nextInodeNumber_ to disk in the following
diff.
Reviewed By: simpkins
Differential Revision: D8192442
fbshipit-source-id: 9b776a73c8d7653002b55985d592b1746e52f878
Summary:
There are a couple benefits to having an explicit Overlay close
method. First, it lets us assert if any code tries to access storage
after EdenMount has started shutting down. Also, it lets us pass
nextInodeNumber_ into Overlay::close() so it can be remembered in the
case of a clean shutdown.
Reviewed By: simpkins
Differential Revision: D8170534
fbshipit-source-id: 14fa607d877df8a3be9e9b0144132c8e8643d98d
Summary:
Before making changes to the way we record the maximum inode number,
cover it with tests.
Reviewed By: simpkins
Differential Revision: D7912059
fbshipit-source-id: 29fdc88bd74c6c07e870017d743d9fd0797baaf3
Summary:
Most filesystems limit path components to 255. To remain consistent,
let's have Eden do the same, at least for write operations.
Reviewed By: simpkins
Differential Revision: D8151020
fbshipit-source-id: 251da94a076f5765111c8e3d9d8a25c37682e2e3
Summary: We are changing `folly::collectAll` to return `SemiFuture` rather than `Future` and this is needed as an interim step. After all calls to `collectAll` are changed to `collectAllSemiFuture`, we'll be renaming it back to `collectAll`.
Reviewed By: yfeldblum
Differential Revision: D8157548
fbshipit-source-id: 27b768ac7ff0d6572bde57f01601045a1fd5d5e5
Summary:
Relax the restriction on changing uid/gid on inodes. We'll see what
cans of worms this opens I guess. (Landing this is low priority, but
might be important for making some of the existing tooling in fbsource
and www work.)
Reviewed By: simpkins
Differential Revision: D7768655
fbshipit-source-id: 95fe02fe7ddc001335dbdb34e16a989a85820240
Summary:
Unify how inode metadata is modified across inode types. This allows
changing the permission bits on a directory.
Reviewed By: simpkins
Differential Revision: D7767254
fbshipit-source-id: 35e9cf652c84c7d8680cc22dec7942e94e9f5af1
Summary:
This moves most inode metadata management into InodeBase and
persists permission bits (and eventually uid/gid) across Eden runs.
Reviewed By: simpkins
Differential Revision: D7035163
fbshipit-source-id: 50145449b56aad1662d53156e6e4960c5f7b6166
Summary: Store tree and file timestamps in the InodeTable so they persist across runs.
Reviewed By: simpkins
Differential Revision: D6891479
fbshipit-source-id: 1c9e6266375aceeaf293a81e73cf7f5334dbc32d
Summary:
This is not at all clear from cppreference.com, but per
https://www.youtube.com/watch?v=dTeKf5Oek2c, it sounds to me like
recommended practice is to either:
`using namespace std::chrono_literals` (or string_literals or
whatever) to pull in a focused set of literals.
or
`using namespace std::literals` to pull in all standard literals.
or
`using namespace std` to pull in everything.
`using namespace std::literals::chrono_literals` is unnecessarily
verbose.
Adopt those standards in Eden.
Reviewed By: simpkins
Differential Revision: D8060944
fbshipit-source-id: 4d9dd4329698b7ff5e5c81b5b28780ca4d81a2a1
Summary:
This allows multiple inodes to update their metadata simultaneously
without contending on InodeTable's locks.
Reviewed By: simpkins
Differential Revision: D7885245
fbshipit-source-id: cc8ab6cd90b7424beec314a115852e08b64026ae
Summary:
Update the folly::Init code to define a `--logging` command line flag, and call
`folly::initLoggingOrDie()` with the value of this command line during
initialization.
This is similar to the existing code that initializes the glog library.
(Programs can use both glog and folly logging together in the same program, and
I expect that many programs will do so as parts get converted to folly::logging
and parts remain using glog.)
Reviewed By: yfeldblum
Differential Revision: D7827344
fbshipit-source-id: 8aa239fbad43bc0b551cbe40cad7b92fa97fcdde
Summary:
While running the secfs filesystem validation tests against Eden, I
discovered a test that caused the eden process to abort. I bisected
and found that D7451330 regressed renaming a directory onto an empty
one. This fixes that case.
Reviewed By: simpkins
Differential Revision: D7945727
fbshipit-source-id: 592ede1b391528c02cd12b2b6ebbf3733fe8f503
Summary:
I got tired of typing PathComponentPiece{"..."} in tests so here are
some operator literals.
Reviewed By: simpkins
Differential Revision: D7956732
fbshipit-source-id: 85d9f3fd725853a54da9e70fc659bd7eb9e0862c
Summary: This old Overlay code is no longer necessary.
Reviewed By: simpkins
Differential Revision: D7903912
fbshipit-source-id: 4a39d6ce7d1f6f81eb13715f2d5d17b22c10d413
Summary:
A persistent (but notably non-durable) mapping from inode
number to a fixed-size record stored in a memory-mapped file. The two
primary goals here are:
1. efficiently (and lazily) reify timestamps for inodes that aren't in the overlay
2. allow the kernel's page cache to drop pages under memory pressure
Reviewed By: simpkins
Differential Revision: D6877361
fbshipit-source-id: a4366b12e21e2bf483c83069cd93ef150829b2ac
Summary:
Make it clear (especially for the upcoming InodeMetadata struct) which
operations with EdenTimestamp and InodeTimestamps will never throw.
Reviewed By: simpkins
Differential Revision: D7920219
fbshipit-source-id: 5917da51b8128455893a1480def6f2c1c8de13d4
Summary:
Per yfeldblum's comment in D7886046, we can use folly::unit instead
of folly::Unit{}. We weren't using folly::unit anywhere, so this diff
replaces folly::Unit{} elsewhere in the Eden code.
Reviewed By: yfeldblum
Differential Revision: D7913462
fbshipit-source-id: fa6ab44ceb406d38713e0f4649224a74e6e51abd
Summary:
Historically, we have seen a number of messages like the following in the Eden
logs:
```
Journal for .hg/blackbox.log holds invalid Created, Created sequence
```
Apparently we were getting these invalid sequences because we were not always
recording a "rename" correctly. The "rename" constructor for a `JournalDelta`
assumed that the source path should be included in the list of "removed" files
while the destination path should be included in the list of "created" files.
However, that is not accurate if the destination path already existed before
the user ran `mv`.
Fortunately, we already check whether the destination file exists in
`TreeInode::doRename()`, so it is straightforward to determine whether the
action is a "rename" (destination does not exist) or an "replace" (destination
already exists) and then classify the destination path accordingly.
As demonstrated by the new test introduced in this commit
(`JournalUpdateTest::moveFileReplace`), in the old implementation,
a file that was removed after it was overwritten would not show up as
removed in the merged `JournalDelta`. Because Watchman relies on
`JournalDelta::merge()` via the Thrift method `getFilesChangedSince()`,
this would cause Watchman to report such a file as still existing even
though it was removed.
This definitely caused bugs in Nuclide. It is likely that other tools that rely
on Watchman in Eden (such as Buck) may have also done incorrect things
because of this bug, so this could explain past reported issues.
Reviewed By: simpkins
Differential Revision: D7888249
fbshipit-source-id: 3e57963f27c5421a6175d1a759db8d9597ed76f3
Summary:
When comparing two source control blob hashes, identical hashes can be assumed
to mean that the file contents are equal. However, differing hashes does not
necessarily mean that the file contents differ. In particular, mercurial
hashes history metadata in addition to the file contents when computing the
blob hash.
This updates Eden to always compare the file contents when the source control
blob hashes differ, rather than assuming that the file contents are different.
Reviewed By: wez
Differential Revision: D7825900
fbshipit-source-id: e611124a66cdd5c44589f20d1d4665a603286530
Summary:
Promote the folly logging code out of the experimental subdirectory.
We have been using this for several months in a few projects and are pretty
happy with it so far.
After moving it out of the experimental/ subdirectory I plan to update
folly::Init() to automatically support configuring it via a `--logging` command
line flag (similar to the initialization it already does today for glog).
Reviewed By: yfeldblum, chadaustin
Differential Revision: D7755455
fbshipit-source-id: 052db34c97f7516728f7cbb1a5ad959def2f6efb
Summary:
To avoid bugs similar to the ones fixed in D7781691, mark a bunch of
folly::Future<folly::Unit> functions with FOLLY_NODISCARD.
Reviewed By: simpkins
Differential Revision: D7782224
fbshipit-source-id: 23ba42aa63011cc33e5a6e18d5bc6d00403a78d3
Summary:
Disallow any kind of mutation operation inside of the .eden directory. We had some
code in place to prevent some of this already, but errors (including EPERM) weren't
passed out from unlink and rename out to FUSE.
Reviewed By: simpkins
Differential Revision: D7781691
fbshipit-source-id: aaecf13779eca75d6ee8765fc8bb3727ce9341de
Summary: The InodeTable work will homogenize mode_t storage, access, and modification across file and tree inodes. In preparation, have InodeBase keep track of the initial mode bits instead of a dtype_t.
Reviewed By: simpkins
Differential Revision: D7031924
fbshipit-source-id: f2e6e4467cecfc0ca06ad998cce0af18a99cc251
Summary: Also print the inode path if the assertions in EXPECT_FILE_INODE fail.
Reviewed By: simpkins
Differential Revision: D7035517
fbshipit-source-id: 6c50acb588d1c985c7e7a1586c06f04a657698f9
Summary:
Now that Eden writes tree data into the overlay even when not
materialized, checkouts can be slow as Eden actively tries to forget
allocated inode data for potentially large trees when
kPreciseInodeMemory is false. This pushes the bulk of that garbage
collection work onto a background thread, making checkouts fast again.
Reviewed By: simpkins
Differential Revision: D7683120
fbshipit-source-id: 2a22e4612a4438b3ed2527670343c49dfcd902bc
Summary: This gets rid of those pesky #ifdefs in FileInode.cpp and TreeInode.cpp.
Reviewed By: wez
Differential Revision: D7735914
fbshipit-source-id: d810461984e21f72670f43ca2d1b4f5aacbf376e
Summary:
This updates the code to store ServerState using a shared_ptr rather than
having it be an inlined member variable of EdenServer.
Previously EdenMount objects contained a raw pointer to the ServerState, with
the reasoning that the EdenServer object should outlive the EdenMount objects.
It turns out that this is not quite true in practice--EdenMount::destroy() will
normally be called before the EdenServer is destroyed, but this may not
actually destroy the EdenMount object immediately.
This fixes a race condition in the FuseTest.initMount() test that could cause
this test to occasionally fail when run on a heavily loaded system.
Reviewed By: chadaustin
Differential Revision: D7720509
fbshipit-source-id: 056ff5985587c8d8c32c11d17ba637ebd7598677
Summary:
I was worried we might leak data in the overlay in this particular
sequence of operations, so here's a test that shows we don't.
Reviewed By: simpkins
Differential Revision: D7663419
fbshipit-source-id: b4b0d08f863fe7b23501e9e4943d18ba0b746e42
Summary:
When we start storing permission changes and other inode
metadata keyed on inode number, we will need to remember inode numbers
before they're accessed to handle the case where the Eden process dies
before the inode is unloaded. Otherwise, data could be left in an
inconsistent state and some writes could be missed.
This change depends on saving (and forgetting) directories being
cheap.
Reviewed By: simpkins
Differential Revision: D7598094
fbshipit-source-id: 9ab127b30a9c09ab62aa08b50c13b3eaa40be60d
Summary:
Split the modifyConflict() test into 3 separate test functions. This test
runs the same checks in 54 separate variations (3 path names, 3 checkout types,
with 6 inode load options).
For each variation we set up a new test mount, creating an overlay and
LocalStore on disk. Simply setting up and tearing down the test mounts can be
expensive if the temporary directory is served from spinning disk. This causes
this test to occasionally time out when run on our continuous build hosts.
Splitting the test up into 3 separate test functions will effectively give it
3x longer to run, since each test function will have a separate timeout. This
will also enable these test functions to run in parallel (although that may not
actually help with performance if they are bottlenecked on disk I/O).
Reviewed By: chadaustin
Differential Revision: D7665409
fbshipit-source-id: 4bddd68e75f38b1b6cc2d57512a5b52855f3bade
Summary:
fdatasync() has a substantial cost and is unnecessary given
our durability guarantees (and the fact that most filesystems on Linux
try to avoid data loss in this common write + rename situation anyway)
Reviewed By: simpkins
Differential Revision: D7641131
fbshipit-source-id: d041e7090dc05a4d4400f86cad9501aa8a6988a9
Summary:
Drop the log level of the "no dir data for inode" message from DBG2 to DBG3 so
we do not have it enabled by default. (Our default log settings are
eden=DBG2.)
Reviewed By: chadaustin
Differential Revision: D7620918
fbshipit-source-id: 25cf30af08054f3ba879bd57f3841b1035e0c75e
Summary:
I'm about to change the code paths through which directories
are saved to and loaded from persistent storage. To help, consolidate
those operations.
Reviewed By: simpkins
Differential Revision: D7597978
fbshipit-source-id: 9cd20027746b0a372a97b7f5daf07c1b54e6b9ce
Summary:
Add a document that begins to sketch out the various transitions that
our inode data structures make so we can reason about their
correctness
Reviewed By: simpkins
Differential Revision: D7434693
fbshipit-source-id: 5478c108c338ccdadc22e864b077195c8be7d1b7
Summary:
Once a mount point has been unmounted we no longer need to care about
outstanding FUSE reference counts--we can treat them as if they are all zero.
This updates EdenMount to tell the InodeMap when the mount point is unloaded,
and changes InodeMap::unloadInode() to make use of this information when
deciding if it needs to remember the inode information.
Previously InodeMap would save information for inodes with outstanding FUSE
reference counts. Writing all of this state to the overlay could take a
non-trivial amount of time.
Reviewed By: chadaustin
Differential Revision: D7555998
fbshipit-source-id: 0896f867ce850ab3e61c262776d536de003685ff
Summary:
Remove the EDEN_HAS_COMMON_STATS checks now that the common/stats stubs have
the required APIs needed by Eden.
Reviewed By: wez
Differential Revision: D7479593
fbshipit-source-id: cc3db50288bfea7aefd6c91391ab800628b7978f
Summary:
Rename `getRefcount()` to `debugGetFuseRefcount()` to make it clear that this
returns the FUSE reference count rather than the pointer refcount, and that
the result should not be used for anything other than debugging or diagnostic
purposes.
There is already a `getFuseRefcount()` method to check the FUSE refcount while
the inode is being unloaded. The only time it is safe to check the FUSE
refcount for programmatic purposes, and `getFuseRefcount()` has a `DCHECK()` to
help ensure that this is the only time it is called.
(`getFuseRefcount()` also predates `getRefcount()`, which was added for the
`eden debug inode` command in D5277870.)
Reviewed By: chadaustin
Differential Revision: D7555997
fbshipit-source-id: e66f2b3d2b9eb14f8f8878d78978851929198a31
Summary:
Change EdenMount to destroy the Overlay object as soon as the `shutdown()`
operation completes. Previously the Overlay would not get destroyed until
EdenMount::destroy() was called.
During graceful restart we transfer mount information to the new process after
`shutdown()` completes, but potentially before `destroy()` has finished. This
previously resulted in a race condition where the new process could start
opening the overlay before the old process had released the lock. This change
should fix that race condition.
Reviewed By: chadaustin
Differential Revision: D7543295
fbshipit-source-id: 82ae33fe6bb0aa9f0a3b010fdd1d350c63442420
Summary: Each Overlay object should be owned only by its EdenMount.
Reviewed By: chadaustin
Differential Revision: D7543294
fbshipit-source-id: 6db40fea31ce298c61d047cba9165887e32926a1
Summary:
Change getScmStatus() so that callers must explicitly specify the commit to
diff against. This should help avoid race conditions around commit or checkout
operations where the parent commit has just changed and eden returns status
information against a commit that wasn't what the client was expecting.
This should still maintain backwards compatibility with older clients that do
not send this parameter yet: we will simply receive the hash as an empty string
in this case, and we still provide the old behavior in this case.
Reviewed By: wez
Differential Revision: D7512338
fbshipit-source-id: 1fb4645dda13b9108c66c2daaa802ea3445ac5f2
Summary:
Now that D7451330 writes saved inode numbers into the overlay, we
need to make sure they get deleted when checking out between trees.
Reviewed By: simpkins
Differential Revision: D7327942
fbshipit-source-id: 9593c7abe9d2e424b9ca1d9c5a5ab8b285867e6e
Summary:
Always remember the inode numbers of children when unloading
a tree. This fixes a bug where, if a tree is unloaded during a build,
child inode numbers would be regenerated, causing the same header to
be compiled once. gcc and clang use inode numbers to decide whether
to #include a file that uses #pragma once, so they must remain
consistent while a mount is up.
This is perhaps a bit controversial because it saves _every_ tree,
unconditionally, to the overlay. I will either stack another diff
that ensures these overlay trees are deleted when checking out to a
new tree or make that change in place here, depending on how urgently
this fix is needed.
Reviewed By: simpkins
Differential Revision: D7451330
fbshipit-source-id: 1c3bc0d55327924c88d1a559d2557cfc158991f2
Summary: Add test demonstrating inode numbers are not consistent across takeover.
Reviewed By: simpkins
Differential Revision: D7300867
fbshipit-source-id: 1e142c28ac00b569b96b5a1867bc8632771c2be9
Summary:
This cleans up the two createOverlayFile() implementations as well as
saveOverlayDir() to all use the same logic for writing out overlay files. This
new logic is more efficient than some of the old code, as we only open the file
once.
Additionally, this also changes the Overlay code to use openat() and related
APIs when accessing files in the overlay, so that we can specify paths relative
the top-level overlay directory. This means that all pathnames are guaranteed
to fit in a small path length known at compile time, and we can avoid ever
allocating path names on the heap.
One potential downside of using openat() is that this functionality may not be
available on Mac OS X, so we will likely still need to provide alternate
implementations that do use heap-allocated absolute paths for Mac support.
Reviewed By: chadaustin
Differential Revision: D7411499
fbshipit-source-id: dd76395130dda4c2b9403cce04f4201f6def0d10
Summary: Add a test case to confirm the behavior of Overlay::getFilePath().
Reviewed By: chadaustin
Differential Revision: D7411498
fbshipit-source-id: f859f29e252c1c296eb236113ad2a8b0a892db37
Summary:
Most of the logic in TreeInode::create(), symlink() and mknod() was very
similar. This adds a TreeInode::createImpl() helper method that these three
functions can share.
This also updates TreeInode::create() to throw EEXIST if the file in question
does exist, rather than using EDEN_BUG(). I believe it is possible (but
unlikely) for this to occur in practice since inode invalidation operations
triggered by a checkout are now processed asynchronously.
Reviewed By: chadaustin
Differential Revision: D7411500
fbshipit-source-id: 85d97995139eee6bff96381561fc28e76d7a2b7c
Summary:
Update all Overlay APIs to accept InodeNumbers and compute the path to the
overlay file internally, rather than requiring the caller to pass in the path
to the file inside the overlay.
Reviewed By: chadaustin
Differential Revision: D7411497
fbshipit-source-id: d057babb0b1b8d6c3b9adf1a21b84eed363dffa1
Summary: After the changes in D7052470 and D7346263, this bit is no longer necessary.
Reviewed By: simpkins
Differential Revision: D7401540
fbshipit-source-id: 04f3ae0376625d4e9584f31932f46774eb5caba4
Summary:
This adds a new LockedState helper class in FileInode to also keep track of
whether or not we are currently holding an refcount on the open file/blob. The
LockedState object automatically decrements this refcount on destruction.
Alternatively, this refcount can be transferred to a new EdenFileHandle object
when unlocking the state.
Previously most of the internal FileInode code simply used EdenFileHandle
objects to manage this refcount. However, this is prone to deadlock, since you
have to ensure that EdenFileHandle objects are never destroyed while the state
is already locked. This new LockedState API makes it harder to have code paths
that may accidentally destroy an EdenFileHandle object while holding the state
lock.
Reviewed By: chadaustin
Differential Revision: D7407423
fbshipit-source-id: 610fcda3220a9f49b734910b7a13e8d68a81a779
Summary:
Update InodeMap::onInodeUnreferenced() to take a pointer to a non-const
InodeBase. This allows the methods it calls, including
InodeBase::updateOverlayHeader() to be non-const.
Using non-const InodeBase objects seems to make sense here since the inode is
in the process of being destroyed.
In a future diff I plan to update FileInode::updateOverlayHeader() to share the
same code as normal file methods to ensure that the overlay file is open. This
modifies the FileInode state's open refcount, so it is useful to have this
method be non-const for that purpose.
Reviewed By: chadaustin
Differential Revision: D7407424
fbshipit-source-id: 541656c7b9b283c5e5650445de5bbdbaae3fc57f
Summary:
Update FuseChannel to send all invalidation requests in a separate thread.
This eliminates a deadlock that could previously occur during checkout
operations. The invalidation requests would block until they could acquire the
kernel's inode lock on the inode in question. However, the inode lock may
already be held by another thread attempting to perform an unlink() or rename()
call. These FUSE unlink or rename operations would be blocked waiting on
Eden's mount point rename lock, which was acquired by the checkout operation.
Checkout operations now let the invalidations complete asynchronously, but we
wait for all invalidation operations to complete before indicating to our
caller that the checkout has succeeded.
Reviewed By: chadaustin, wez
Differential Revision: D7404971
fbshipit-source-id: 6fa20c00d054e210eb0258d247d083010557f210
Summary:
Decouple inode number assignment from materialization status.
The idea is that we will always assign entries an inode number and
track whether an entry is materialized otherwise. This is necessary
to give consistent inode values across remounts.
Reviewed By: simpkins
Differential Revision: D7052470
fbshipit-source-id: 80d3f2a2938463198a3132182537e6223c79d509
Summary:
Verify Eden handles looking up an inode by number after graceful
restart and checkout.
Reviewed By: simpkins
Differential Revision: D7346263
fbshipit-source-id: 876b4837708da9ac31f72c06e7defc797fe126f3
Summary: Small refactoring prior to a fix in a later diff.
Reviewed By: simpkins
Differential Revision: D7358218
fbshipit-source-id: 198160df1aa0dc65b917c5a96c2998c3675b03fe
Summary: Now we can write unit tests for takeover. :)
Reviewed By: simpkins
Differential Revision: D7345419
fbshipit-source-id: 05741dab65016c59109bf190c83be2f676a6141d
Summary:
This replaces the `ensureDataLoaded()`, `materializeForWrite()`, and
`materializeAndTruncate()` methods in `FileInode` with new safer alternatives:
`runWhileDataLoaded()`, `runWhileMaterialized()`, and `truncateAndRun()`
These new methods take a function to run, and run it while holding the state
lock with the FileInode guaranteed to be in the desired state. They also
require the caller move in the state lock as an argument, to help ensure that
the caller gets the locking behavior correct.
The old methods required that the caller not already hold the state lock while
calling them. This pre-condition was violated in `FileInode::write()` and
possibly other places. They also required releasing the lock between they
confirmed the file state and when the caller's function ran, requiring
additional lock and unlock operations, and also making the code harder to
reason about since other threads could change the state during the time when
the lock was released.
Reviewed By: chadaustin
Differential Revision: D7363180
fbshipit-source-id: d8e667d0bc7006c519252a8d0682af97517997eb
Summary:
The changes in this diff comments out unused parameters. All changes are automated using clang-tidy.
This will allow us to enable `-Wunused-parameter` as error.
Reviewed By: simpkins
Differential Revision: D7371610
fbshipit-source-id: 0134e2f0b916313d690c073a46d747c52399a226
Summary:
FileInode::read() did not handle being called in the BLOB_LOADING or NOT_LOADED
states. This method is only called from EdenFileHandle::read(), which does no
checking that it is fully loaded first.
This fixes read() to always call ensureDataLoaded() first.
Reviewed By: chadaustin
Differential Revision: D7359878
fbshipit-source-id: f5a1c8a28db3267da3180b67f970430e3ea291da
Summary:
While debugging a takeover crash (task #25590050) we wanted to help confirm
that inode objects weren't ever getting copied somehow. That turned out not to
be the issue, but it still seems worth explicitly deleting the copy and move
constructors and assignment operators for InodeBase.
Reviewed By: wez
Differential Revision: D7342050
fbshipit-source-id: ae07214072bfb8353584a07066aae2dc1adf2797
Summary:
Update the getScmStatusBetweenRevisions() thrift call to use the new
diffCommits() function that diffs source control Tree objects without creating
TreeInode objects to perform the diff.
This addresses two bugs:
- Each call to EdenMount::diffRevisions() constructed a new root inode
associated with the mount, and this would never get destroyed. It was not
destroyed at the end of the diffRevisions() call since inodes are normally
not destroyed immediately when they are unreferenced. It was not destroyed
during EdenMount::shutdown() since EdenMount didn't have any references to
these additional root inode structures and their children.
- EdenMount::diffRevisions() incorrectly swapped ADDED and REMOVED statuses in
the result.
Reviewed By: wez
Differential Revision: D7341609
fbshipit-source-id: 16e755a0ff685f51c977c3b27d6af96908f33494
Summary:
Add a function for diffing two source control commits without needing to
instantiate TreeInode objects.
Reviewed By: wez
Differential Revision: D7341604
fbshipit-source-id: 557eef87faa2785ab96d51b09569a46f892a71f6
Summary:
Fix the code to generate exceptions based on an errno error using
std::generic_category rather than std::system_category.
Reviewed By: yfeldblum
Differential Revision: D7329997
fbshipit-source-id: 3fe257bbbc7a631c801f31120592c8bdbc25c8bf
Summary:
Add a new utils/SystemError.h header with helper functions to check if a
`std::system_error` contains an errno value.
Most of the code in Eden previously only checked for `std::system_category`
when looking for errno values. `std::generic_category` is the correct category
to use for errno exceptions, but folly/Exception.h incorrectly throws them as
`std::system_category` today. This change makes Eden treat either error type
as errno values for now.
Reviewed By: yfeldblum
Differential Revision: D7329999
fbshipit-source-id: 67a3c3ea10371c53a2e34236b7575deac4cbd53a
Summary:
This helps distinguish `facebook::eden::FileHandle` from
`facebook::eden::fusell::FileHandle`, and will make it possible to eliminate
the separate `facebook::eden::fusell` namespace in the future.
Reviewed By: chadaustin
Differential Revision: D7314456
fbshipit-source-id: 0869427766fe666d119a59c7df1c97825a8ad36e
Summary:
This test exercises code under eden/fs/fuse, so put it with that directory's
tests.
Reviewed By: chadaustin
Differential Revision: D7314457
fbshipit-source-id: b664aaa3086b2e65f7ae8b76ae71878f39fd9d17
Summary:
There's some code that uses Entry::hasInodeNumber to decide whether an
inode is in the process of being loaded. This change is a
prerequisite for D7052470.
Reviewed By: simpkins
Differential Revision: D7311182
fbshipit-source-id: e04a89c814950e6ce33e73b963b3ffb0f0e353b8
Summary:
Remove the `FuseChannel::stealFuseDevice()` method, and instead change the
session completion future to return the FUSE device FD if it is still valid.
This ensures we only extract the FUSE device when it is still valid: if an
unmount event is being processed by a FUSE worker thread simultaneously with a
call to `FuseChannel::takeoverStop()` the session completion future will return
an empty `folly::File` since the FUSE device has been shut down.
This also helps clarify the synchronization semantics around modification to
the `fuseDevice_` member variable.
Reviewed By: wez
Differential Revision: D7300847
fbshipit-source-id: 02ced8fc9d24e7cd526d911782949d0bfbf0e5a7
Summary:
The FuseChannel destructor previously could be invoked while there are still
outstanding requests, which would cause problems when responses were generated
for these requests.
This makes the FuseChannel destructor private, requiring users to always
allocate FuseChannel objects on the heap and destroy them using a destroy()
method. The destroy() method still blocks waiting for the FUSE worker threads
to exit, but if there are still outstanding FUSE requests it defers the
deletion of the FuseChannel until they are complete.
The EdenMount class currently couldn't ever destroy FuseChannel objects with
outstanding requests, since it waits for the completion future to fire before
invoking the destructor. Fixing this behavior is mostly for correctness of the
FuseChannel API and for the benefit of our unit tests.
Reviewed By: chadaustin
Differential Revision: D7297829
fbshipit-source-id: 643d1332be84a1f25ee30ba2a2ea3515806f00ef
Summary:
Make ServerState also store the clock as a member variable. This moves more
server-wide state into the ServerState class, and allows us to eliminate
another argument to the EdenMount constructor.
Reviewed By: chadaustin
Differential Revision: D7297832
fbshipit-source-id: 2a063d67752f46686987390b3faefb304455568a
Summary:
Move the EdenCPUThreadPool from EdenServer to ServerState.
This ensures that all EdenMount objects always have a thread pool. Previously
most unit tests were creating an EdenMount without a thread pool set, but the
FileInode code and TreeInode code could end up wanting to use the thread pool
even if no FuseChannel is running.
Reviewed By: chadaustin
Differential Revision: D7297828
fbshipit-source-id: 9861b3439cd091afe31fa13f449aa14656983318
Summary:
Remove the `FuseChannel::getSessionCompleteFuture()` method and instead have
`initialize()` and `initializeFromTakeover()` return the `Future` that can be
used to wait on session completion.
This makes it explicit from an API perspective that this session completion
future will only be invoked if initialization is successful. It also
eliminates the possibility of anyone calling getSessionCompleteFuture() more
than once.
I also changed the session completion future to use a folly::SemiFuture rather
than a folly::Future, to make it explicit that callers generally should not
execute their callback inline in the same thread where the SemiFuture is
fulfilled.
Reviewed By: chadaustin
Differential Revision: D7297835
fbshipit-source-id: 3a1157951f0738f1692833ed5e875c3e9c6d6d69
Summary: Testing convenience. The NO_CHECK constructor is no longer necessary now that simpkins made the original constructor constexpr.
Reviewed By: simpkins
Differential Revision: D7295539
fbshipit-source-id: 717f5a7a8d3d9790966fad441da7b3308495e2c3
Summary: Cleaning up the takeover initialization code for EdenMount and InodeMap.
Reviewed By: simpkins
Differential Revision: D7294419
fbshipit-source-id: 58506f04259bb1017e6cd2e80e40e5820de6200f
Summary:
Instead of having a rule that save() must be called after
InodeMap::shutdown, just have InodeMap::shutdown return the
SerializedInodeMap if it's desired.
Reviewed By: simpkins
Differential Revision: D7292773
fbshipit-source-id: 2ba35fc729e19af122fe5d6c5a3287ad6b8af946
Summary:
This changes FuseChannel to fulfill the session complete future immediately in
the thread where it finishes. This may occur inside a FUSE worker thread.
It is now up to the caller to use `via()` if they want their callback to run in
a separate thread.
This simplifies the FuseChannel code and also addresses a crash that occurs in
the unit tests sometimes: If the FuseChannel destructor is invoked without
explicitly invoking the FuseChannel, it would schedule the session complete
promise to be fulfilled in a separate EventBase thread. However, the promise
may then have already been destroyed by the time it can run in the EventBase
thread.
There are still additional synchronization issues to fix in the FuseChannel
tests, but this resolves one problem.
Reviewed By: chadaustin
Differential Revision: D7282001
fbshipit-source-id: be64d3ef6a0e664ed7a2cf93a549acc140184095
Summary:
Empty .gitignore files should be processed normally, and treated as no new
ignore rules. The logic in D6659654 had accidentally introduced a bug where we
would completely skip processing any directories that contained empty
.gitignore files.
Reviewed By: wez
Differential Revision: D7261276
fbshipit-source-id: 033e199f15d7763bff5f9a080a226c50e481aa9d
Summary:
- Update FuseChannel::initializate() to not require an Executor. Rather than
performing initialization using the supplied Executor, it simply starts one
worker thread first and performs initialization there.
- Change the API semantics slightly so that the session complete future is
invoked only if initialization succeeds. Previously the session complete
future could be invoked if initialization failed as well.
- Replace the activeThreads counter with a stoppedThreads counter to determine
when the session complete future should be invoked. The previous
activeThreads behavior was somewhat racy, since the activeThreads counter was
incremented inside each worker thread, but most places that checked it did
not wait to ensure that all worker threads had successfully incremented it
yet.
Reviewed By: chadaustin
Differential Revision: D7243910
fbshipit-source-id: 93b3465509bd9bf6fa90ea097e70dac3193172f9
Summary: It's a little surprising to me the clang build passed without these explicit template instantiations!
Reviewed By: simpkins
Differential Revision: D7253536
fbshipit-source-id: 2f48d5571777f4e978b6947183eefb03158d5014
Summary:
Split up the FuseChannel and EdenMount APIs for starting a brand new FUSE
channel vs taking over an existing channel.
These operations are somewhat different as initializing a new FUSE channel
requires performing a negotiation with the kernel that will not complete
immediately. Therefore these APIs return a Future object. Taking over an
existing FUSE channel is always an immediate operation, and therefor does not
need to return a Future object.
Reviewed By: chadaustin
Differential Revision: D7241997
fbshipit-source-id: 22780f2df76b2d554ab2a4043b6425fa7a4e9c94
Summary:
Ensure that we ignore SIGPIPE in unit tests. The
`FuseChannel::requestSessionExit()` function sends SIGPIPE to its worker
threads, and expects this to wake up the worker threads if they are stuck
inside a read() call.
In normal program builds folly/Subprocess.cpp causes SIGPIPE to be ignored on
startup. However, Subprocess.cpp is not necessarily linked into all of our
unit tests. I haven't tracked down exactly why, but SIGPIPE in the unit tests
seems to kill the entire process only in opt-ubsan builds.
In the future we probably should clean up how requestSessionExit() triggers its
worker threads to exit: the current code is still potentially racy, since the
worker threads could receive the SIGPIPE just after checking `sessionFinished_`
and before calling `read()`, in which case they may go back to sleep rather
than exiting. However for now this should be a simple fix to get the tests
passing in all build modes.
Reviewed By: wez
Differential Revision: D7224370
fbshipit-source-id: a56fe3331fc5aa6a49ccbe6b0678b479a44ffc07
Summary:
To make it clearer to me why all the calls to newPtrLocked were safe, and to
eliminate some duplication, I captured the newPtrLocked call patterns into
member functions on LoadedInode and TreeInode::Entry.
Reviewed By: simpkins
Differential Revision: D7207542
fbshipit-source-id: 25de77e72c0898be43b3fbdddab835d64101755e
Summary:
To make it clearer why newPtrLocked is safe during the Inode class
construction process, add a takeOwnership function (better name
suggestions welcome) that converts from the newly-instantiated
unique_ptr to a managed InodePtr.
Reviewed By: simpkins
Differential Revision: D7204818
fbshipit-source-id: 8a40189588442490120623da86195c6fc99c9c51
Summary:
This diff is mostly preparation. It's submitted separately to
remove mechanical refactoring noise from the following diff, where
every entry is assigned an inode number upon construction.
Reviewed By: simpkins
Differential Revision: D7116832
fbshipit-source-id: 2943c45340a9a751eb52bf13e19d233d829494c0
Summary:
If the root TreeInode wants to allocate inode numbers, the inode
allocator must be initialized first. But complete InodeMap
initialization requires the root TreeInode. So split this into two
parts.
Also, I changed the inode allocator to a single atomic increment instead
of a lock acquisiton.
Finally, the extra assertions in this diff uncovered what looks like a
bug in the takeover logic where nextInodeNumber_ could end up being
smaller than the value in the takeover data, since the max inode
number from the overlay was assigned after loading from takeover data.
Reviewed By: simpkins
Differential Revision: D7107706
fbshipit-source-id: ec43cc81c11d709261598739c622609b372433a2
Summary:
I spent way too long trying to figure out why my refactorings were
causing invariant errors inside InodeMap. It turns out that we
initialize the root TreeInode before InodeMap::initialize is called,
which I suspect resulted in duplicate inode numbers being handed out.
Reviewed By: simpkins
Differential Revision: D7106302
fbshipit-source-id: b459734fb96bfbb6b4b27a1d23de8b6406d30ca4
Summary:
This adds a FakeFuse class to simulate a FUSE connection to the kernel, but
that is actually communicating with our userspace test harness code. This also
adds a FakePrivHelper class which implements the PrivHelper interface but
returns FakeFuse connections rather than real FUSE connections to the kernel.
This will enable us to write unit tests that exercise more of the FUSE and
EdenMount logic, and will also enable us to test error conditions and ordering
behaviors that are difficult to reliably reproduce with a real FUSE mount.
This also includes some very basic tests using this new code. The code in
fuse/test/FuseChannelTest.cpp creates a FuseChannel using a FakeFuse object,
and the code in inodes/test/FuseTest.cpp creates a full EdenMount object using
FakePrivHelper and FakeFuse. The tests are pretty similar for now, and only
exercise the FUSE initialization code. I will expand these tests in subsequent
diffs.
Reviewed By: wez
Differential Revision: D7050826
fbshipit-source-id: 4f82375d65664ca3a5a228a577caa4d1d47454fe
Summary:
I'm seeing test failures that I have not yet understood and I
thought they might be caused by an implicit conversion from
fusell::InodeNumber to bool. Well, they're not, but this is how I
discovered that. I'm not sure I want to land this change, but I'm
going to leave it around until I figure out what's happening with my
other diffs.
Reviewed By: simpkins
Differential Revision: D7077635
fbshipit-source-id: 50bf67026d2d0da0220c4709e3db24d841960f4b
Summary:
I am working on a stack of diffs that changes how we allocate inode
numbers to tree entries. I was hitting test failures I could not
understand, so in the process of trying to understand the flows
through InodeMap, I found newChildLoadStarted to be redundant with
shouldLoadChild.
Note: Today, allocateInodeNumber() acquires the InodeMap's lock, but
in a later diff, inode numbers will be assigned en masse during
TreeInode construction.
Reviewed By: simpkins
Differential Revision: D7059719
fbshipit-source-id: 624b861040d585d2cae41d7ec2aae7d528ff8336
Summary:
Previously we returned an ENOENT error in response to a FUSE lookup() call for
a name that does not exist. However, this does not allow FUSE to cache the
result, so we will continue to receive lookup() calls for this path in the
future.
This changes EdenDispatcher to return a successful response with an inode
number of 0 instead. This tells the kernel that the name does not exist, but
allows the kernel to cache this negative lookup result (for as long as
specified in the entry_valid field in the response).
Reviewed By: wez
Differential Revision: D7076811
fbshipit-source-id: a2b9977e58d6b6eecb584699b9d93b5ad29ad5ad
Summary:
Update the TreeInode code to always update the FUSE cache whenever we add a new
entry to the directory.
Up to now we have been fine without this since the kernel never cached negative
lookup responses for us. In order to turn on FUSE caching of negative lookup()
responses we need to invalidate the cache whenever we create a new entry.
- Have create(), symlink(), mknod() and mkdir() invalidate the FUSE entry cache
if they are not being invoked from a FUSE request. These are almost always
invoked from a FUSE request, but flushing the cache is the correct thing to
do if we add code in the future that can trigger these from a thrift call or
via some other non-FUSE code path.
- Change the checkout code to invalidate the FUSE cache whenever it creates new
children entries.
Reviewed By: wez
Differential Revision: D7076812
fbshipit-source-id: d6489995b415260e84b3701c49713b0ef514f85d
Summary:
This removes the TARGETS files from the eden github repository. The
open source buck build has been failing for several months, since buck
removed support for the thrift_library() rule.
I will potentially take a stab at adding CMake build support for Eden
at some point in the future.
Reviewed By: chadaustin
Differential Revision: D6893233
fbshipit-source-id: e6023094a807cf481ac49998c6f21b213be6c288
Summary: Per code review comments on D6983198, this simplifies the way we check if mode bits have changed in a meaningful-to-source-control way.
Reviewed By: simpkins
Differential Revision: D7015339
fbshipit-source-id: 548ead337fbea1c1dcb72b880921671e9b6188ac
Summary:
mode_t isn't really part of a TreeEntry and I also wanted to see all
the places where we convert an entry type from source control into
mode bits.
Reviewed By: simpkins
Differential Revision: D6983198
fbshipit-source-id: ce1d0976f5fc5130c34a8c93c07a4e26a7cdaf71
Summary:
This is the type of a tree entry, which may be another tree, so
FileType is not an accurate name.
Reviewed By: simpkins
Differential Revision: D6981168
fbshipit-source-id: 997eb8a27f599310ed678ce221c8083722db8bff
Summary:
While we're changing how timestamps are stored, we might as well
implement this small-ish efficiency. Instead of storing timestamps as
a timespec (16 bytes), store them as 64-bit nanoseconds with a range
slightly larger than what ext4 supports. Assuming a million inodes,
this saves 24 MB.
This diff introduces the EdenTimestamp type. The next diff will start
using it.
Reviewed By: simpkins
Differential Revision: D6957659
fbshipit-source-id: 4551af6f5b8c1ff610ba88795f69e7d69d7f605d
Summary:
I want to rename FileType to TreeEntryType so I removed this one first
and replaced all of its uses with an isTree() method.
Reviewed By: simpkins
Differential Revision: D6980501
fbshipit-source-id: 105b8c599585e63efd44043e761db40e2824e77e
Summary:
Our Model TreeEntry code was a bit too general - in reality, both git
and hg only support a handful of specific tree entries: regular files,
executable files, symlinks, and trees. (git also supports
submodules.) This diff delays the expansion of a TreeEntry's type
into a full mode_t.
Reviewed By: simpkins
Differential Revision: D6980003
fbshipit-source-id: 73729208000668078a180b728d7e0bb9169c6f3c
Summary: I should have caught this before...
Reviewed By: wez
Differential Revision: D6995395
fbshipit-source-id: 08efd0aacb051f2b1754930b311e3a42afc3c538
Summary:
Another small refactoring that I don't want mixed into my
bigger diffs.
Reviewed By: wez
Differential Revision: D6927482
fbshipit-source-id: 28cc60dfdffb50921a5ef9cca4e2814b90d3b701
Summary:
There were places we were acquiring a lock unnecessarily. In
addition, I'm looking at reducing the number of places where we store
the full mode_t to see if we can get away with dirtype_t or something
similar.
Reviewed By: wez
Differential Revision: D6972140
fbshipit-source-id: bb29a4473f3056e39596600d22e67374ca484735
Summary:
Update FileInode so that getattr() and setattr() both return a reasonable value
in st_blocks.
Previously we always returned 0 in st_blocks, which caused applications like
`du` to always report files as using 0 space in Eden mounts. Now we compute
st_blocks based on st_size, so that `du` will report reasonable estimates for
when scanning the size of subdirectories inside an Eden mount.
Reviewed By: chadaustin
Differential Revision: D6932098
fbshipit-source-id: bd29e46821176e510f420e6e2b6ce480b80d50ff
Summary:
Move all of the privhelper functionality into a PrivHelper class. The
ServerState object now stores the PrivHelper object to use, rather than having
a global singleton.
This will make it easier to stub out the PrivHelper functionality during unit
tests.
Reviewed By: wez
Differential Revision: D6929862
fbshipit-source-id: e3edcb0a03ba9afdf34554cb961fd74557cdd6e3
Summary:
Update EdenMount::diff() to use the UserInfo object stored in the shared
ServerState rather than calling UserInfo::lookup() on each diff operation.
Reviewed By: wez
Differential Revision: D6929865
fbshipit-source-id: a68ab1fa9eb345b59972e67c3aac258b4dbcdab5
Summary:
Add a new ServerState class to store process-wide state that is shared across
multiple mounts. Up until now we have passed around the shared state data as
separate variables.
This is intentionally separate from the existing EdenServer class to allow unit
tests to create EdenMount objects without an EdenServer object.
Reviewed By: wez
Differential Revision: D6929861
fbshipit-source-id: 5f22efb6d79dfd70031be1dc37f494c2ad8af902
Summary:
We no longer need this and I believe that this was contributing
to this source of flakiness in our CI; this stack trace triggers when
we get an aborted or short read from the kernel:
```
*** Aborted at 1517827659 (Unix time, try 'date -d 1517827659') ***
*** Signal 11 (SIGSEGV) (0x2d0) received by PID 153706 (pthread TID 0x7f63ea37d700) (linux TID 160573) (maybe from PID 720, UID 0) (code: address not mapped to object), stack trace: ***
@ 0000000002abfa2d folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
./folly/experimental/symbolizer/SignalHandler.cpp:413
@ 00007f646e96dacf (unknown)
@ 00007f646e96aa20 __pthread_kill
@ 0000000001873748 facebook::eden::fusell::FuseChannel::requestSessionExit()
./eden/fs/fuse/FuseChannel.cpp:483
@ 000000000187d2f7 facebook::eden::fusell::FuseChannel::processSession()
./eden/fs/fuse/FuseChannel.cpp:656
@ 000000000187dab9 facebook::eden::fusell::FuseChannel::fuseWorkerThread(unsigned long)
./eden/fs/fuse/FuseChannel.cpp:493
@ 00007f646f267170 execute_native_thread_routine
@ 00007f646e9637a8 start_thread
@ 00007f646e047a7c __clone
```
my theory is that we're allowing shutdownImpl to free things out from under other
threads before they've all seen this signal and wound down fully. This is slightly
speculative in that I haven't managed to reproduce this stack trace on my devserver.
We don't really need this additional signal any longer.
Reviewed By: simpkins
Differential Revision: D6907734
fbshipit-source-id: 0f0138b631a7201fc9a4a1c93c2cde846e869cbd
Summary: Small refactoring I should have done with the previous diff.
Reviewed By: simpkins
Differential Revision: D6927152
fbshipit-source-id: 1dcda01134c3d63c62169c5728dba24ca0eebd68
Summary:
Update `TreeInode::diff()` to check if its hash matches the source control tree
it is being compared to, and return early if they are identical.
I'm surprised that I forgot to include this initially when implementing
`TreeInode::diff()`
This makes `hg status` faster when a large number of unmodified directories
have been loaded.
Reviewed By: chadaustin
Differential Revision: D6890615
fbshipit-source-id: 561630d0220b4875dbf3678161cdb41a8aa4fc82
Summary:
This re-orders some of the code in `TreeInode::diff()` slightly. This should
not affect the behavior of the code.
This moves the `isIgnored` check inside the main `contents_.wlock()` block.
This reduces the number of places where we grab the lock, and will help keep
things simple for an upcoming diff where I need to add some more checks in this
code with the lock held.
This also changes `inodeFuture` to use the new `Future::makeEmpty()`
constructor rather than having to use an `Optional<Future>`
Reviewed By: chadaustin
Differential Revision: D6890616
fbshipit-source-id: 354bbf6a6be6d356fd23e6c0fb6b534679bbe0bb
Summary: Small things I've needed in later diffs.
Reviewed By: wez
Differential Revision: D6877755
fbshipit-source-id: c9002eb0b92dbd8fe9c4f636d2ca79b25cde331f
Summary:
While working on timestamp storage, the fact that
InodeTimestamps was a member of InodeBase kept getting in the way.
Make it its own type.
Reviewed By: simpkins
Differential Revision: D6862835
fbshipit-source-id: 91d8984764f0586b9fa52e961eb5606a530e0416
Summary: This API seems like it should be const, as it does not modify the clock.
Reviewed By: chadaustin, zhupanov
Differential Revision: D6869719
fbshipit-source-id: c8bf4ccab34538b59e6baeedd0b0ff88b328236e
Summary:
This diff moves the mount-time initialization handling
out of the main loop. This rationale for this is:
* We don't (and shouldn't!) need to process FUSE_INIT for takeover
processing, and this structure allows us to make stronger assertions
about our state.
* we can avoid spinning up multiple threads in the (rare!) case that
the FUSE_INIT fails
* It is now a little harder for exceptions during initialization to
escape our notice.
In rearranging this stuff, I found a race condition in the worker thread
shutdown; we could erroneously emit a completion event before all of
the threads had been torn down and this resulted in sporadic integration
test failures hitting the assertion for the number of joined threads
in the destructor.
Reviewed By: simpkins
Differential Revision: D6766330
fbshipit-source-id: 32afb5a7c739c75aebfdb0a8f896eec5f41ad33f
Summary:
Dir's contents were represented as a vector of 64-bit
pointers to 48-byte structs. This change removes that layer of
indirection, reducing memory usage and slightly pessimizing insertion.
The diff is mostly mechanical outside of the TreeInode.h changes and
calls to emplace..
I'll run memory tests tomorrow, though it's a gamble as to whether
private bytes will show a difference. I may need to shrink the Entry
struct too.
Reviewed By: wez
Differential Revision: D6804957
fbshipit-source-id: b126656dbc7951565e74b6401adde6353e809056
Summary:
Tiny thing I noticed when reading code. Keep the entry name
as a StringPiece rather than bouncing through char*.
Reviewed By: zhupanov
Differential Revision: D6820080
fbshipit-source-id: 884e55f74094f44012efbe44b86d8e5903300967
Summary:
isSameAs calls getSha1 which was failing on symlinks. The
original concern was that asking for the SHA-1 of a symlink is
ambiguous: do you want the hash of the symlink or the target? But we
already check for whether you are requesting the SHA-1 of a symlink in
EdenServiceHandler, so it's redundant and incorrect to check in
FileInode too.
Reviewed By: simpkins
Differential Revision: D6847489
fbshipit-source-id: 13966da06bcde75c5c568e09fef14e735de47cfb
Summary:
It's interesting to see the total number of loaded files
vs. trees when the loaded inode count is high.
Reviewed By: wez
Differential Revision: D6765874
fbshipit-source-id: 178b30184428bd5cf5e005eb475e4f5a1476c385
Summary:
I'm not sure what was wrong with the old code, but I
simplified and clarified all of the time math and now `eden debug
unload` behaves as I'd expect it should.
Reviewed By: simpkins
Differential Revision: D6764962
fbshipit-source-id: 3ed359d4ab4652e95d1538a0982c24185999351c
Summary:
rdev doesn't add any value yet. We can add it back if we want
to implement support.
Reviewed By: simpkins
Differential Revision: D6792346
fbshipit-source-id: ce16317074f1daa456737c55804da8fb7f2b7a94
Summary:
This slightly improves the code in EdenMount by having
just a single call to the FuseChannel constructor with an optional
`fuse_init_out` instance that we can use to determine if we are
performing a takeover or not.
Reviewed By: simpkins
Differential Revision: D6766328
fbshipit-source-id: ece140a1572e2934a3e35bfe25b83af910346c18
