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:
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:
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:
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:
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: This gets rid of those pesky #ifdefs in FileInode.cpp and TreeInode.cpp.
Reviewed By: wez
Differential Revision: D7735914
fbshipit-source-id: d810461984e21f72670f43ca2d1b4f5aacbf376e
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:
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:
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:
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:
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:
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: 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:
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: Small refactoring I should have done with the previous diff.
Reviewed By: simpkins
Differential Revision: D6927152
fbshipit-source-id: 1dcda01134c3d63c62169c5728dba24ca0eebd68
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:
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:
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: Minor stylistic changes that were done during constification but factored out.
Reviewed By: chadaustin
Differential Revision: D6774976
fbshipit-source-id: d18cd339153cf16ff69be0de5f3eb019a4baa1a0
Summary: A prior shelve contained this fix but never got landed.
Reviewed By: wez
Differential Revision: D6676206
fbshipit-source-id: b8c733be663ff56e1a0625f09ec505891d430084
Summary:
It is no longer correct to assert that state->file is set if O_TRUNC happened
before blob import from hg finished. It surprises me we never saw a crash
because of that. Also, the O_TRUNC path after blob import finishes can never
complete a future, so don't try.
Reviewed By: wez
Differential Revision: D6656699
fbshipit-source-id: 5e245fc46185714e5f5d81c2680835a3497747ff
Summary:
Today, if a file is ever opened for read, each FileInode keeps a copy
of the data as long as the FileInode is around. This results in
excessive memory consumption under common mistakes like repo-wide grep
or `hg revert .`.
I will audit all of the state machine transitions and blob accesses
before landing this diff.
Reviewed By: wez
Differential Revision: D6598957
fbshipit-source-id: 1eb4aeb08057ce993a29a86d298e153675fee4a1
Summary:
This came up when I was auditing the rules about when it's
safe to read from a FileInode. read() must only be called while openCount > 0.
Reviewed By: wez
Differential Revision: D6604898
fbshipit-source-id: 829ddc335bd58201c2b456ee544cdc6253ebf66c
Summary:
In a follow-on diff, the constraint will be that state->blob
will only be guaranteed valid after ensureDataLoaded() while a
FileHandle is alive. Thus, ensureDataLoaded() must return a
FileHandle.
Reviewed By: wez
Differential Revision: D6586237
fbshipit-source-id: ccc269d322b8c725c93145df5de2add9a2b90207
Summary:
This serves a few purposes:
1. We can avoid some conditional code inside eden if we know that
we have a specific fuse_kernel.h header implementation.
2. We don't have to figure out a way to propagate the kernel
capabilities through the graceful restart process.
3. libfuse3 removed the channel/session hooks that we've been
using thus far to interject ourselves for mounting and
graceful restarting, so we were already effectively the
walking dead here.
4. We're now able to take advtange of the latest aspects of
the fuse kernel interface without being tied to the implementation
of libfuse2 or libfuse3. We're interested in the readdirplus
functionality and will look at enabling that in a future diff.
This may make some things slightly harder for the more immediate
macOS port but I belive that we're in a much better place overall.
This diff is relatively mechanical and sadly is (unavoidably) large.
The main aspects of this diff are:
1. The `fuse_ino_t` type was provided by libfuse so we needed to
replace it with our own definition. This has decent penetration
throughout the codebase.
2. The confusing `fuse_file_info` type that was multi-purpose and
had fields that were sometimes *in* parameters and sometimes *out*
parameters has been removed and replaced with a simpler *flags*
parameter that corresponds to the `open(2)` flags parameter.
The *out* portions are subsumed by existing file handle metadata
methods.
3. The fuse parameters returned from variations of the `LOOKUP` opcode
now return the fuse kernel type for this directly. I suspect
that we may need to introduce a compatibility type when we revisit
the macOS port, but this at least makes this diff slightly simpler.
You'll notice that some field and symbol name prefixes vary as
a result of this.
4. Similarly for `setattr`, libfuse separated the kernel data into
two parameters that were a little awkward to use; we're now just
passing the kernel data through and this, IMO, makes the interface
slightly more understandable.
5. The bulk of the code from `Dispatcher.cpp` that shimmed the
libfuse callbacks into the C++ virtual methods has been removed
and replaced by a `switch` statement based dispatcher in
`FuseChannel`. I'm not married to this being `switch` based
and may revise this to be driven by an `unordered_map` of
opcode -> dispatcher method defined in `FuseChannel`. Regardless,
`Dispatcher.cpp` is now much slimmer and should be easier to
replace by rolling it together into `EdenDispatcher.cpp` in
the future should we desire to do so.
6. This diff disables dispatching `poll` and `ioctl` calls. We
didn't make use of them and their interfaces are a bit fiddly.
7. `INTERRUPT` is also disabled here. I will re-enable it in
a follow-up diff where I can also revise how we track outstanding
requests for graceful shutdown.
8. I've imported `fuse_kernel.h` from libfuse. This is included
under the permissive 2-clause BSD license that it allows for
exactly this integration purpose.
Reviewed By: simpkins
Differential Revision: D6576472
fbshipit-source-id: 7cb088af5e06fe27bf22a1bed295c18c17d8006c
Summary:
Easy to overlook this; the issue is that we need to explicitly
do something about the error case when we're stitching together Promises
by hand, otherwise we will silently drop exceptions.
Flat manifest imports are failing in `RestartTestHg` in master
at the moment. That error was silently being swallowed and the test would
hang until it timed out.
This is an uglyish hack to explicitly propagate the error condition so that
that test will error out.
This diff doesn't fix the source of the manifest import issue; that is addressed
in the next diff (turned out to be that the `--takeover` flag was not being
passed correctly)
Reviewed By: bolinfest
Differential Revision: D6627973
fbshipit-source-id: b7093890f543618a11682e939f8802f1309831d4
Summary: Added type identification capability to InodeBase and its descendants FileInode and TreeInode.
Reviewed By: simpkins
Differential Revision: D6564902
fbshipit-source-id: ce9300102d6d6d1c42616eb1e32042f21f6e6cce
Summary:
Add EdenCPUThreadPool and UnboundedQueueThreadPool types to make it clearer
that it's always okay for prefetch, deferred diff entry, and hg import to
shuttle work back to the main thread pool.
This diff changes no behavior - it just makes some invariants explicit.
Reviewed By: wez, simpkins
Differential Revision: D6504117
fbshipit-source-id: 3400ad55c00b3719ecba31807fd992442f622cd9
Summary:
There's no technical reason to block an open() request until the data
load / materialization returns. This change returns immediately from
open() and then waits if necessary in a subsequent write() call.
Reviewed By: wez
Differential Revision: D6391486
fbshipit-source-id: 862f87e3c3a0d760bacb0f8ca7acc479037fec2f
Summary:
Follow-up to comments in D6466209. All access to the clock goes
through the Clock interface, making time deterministic in unit tests.
Reviewed By: simpkins
Differential Revision: D6477973
fbshipit-source-id: 24e51bdb52d0d079b34d91598d2e787d361f2525
Summary:
open() called materializeInParent unconditionally, and setattr never
called it, implying it was possible to truncate a file without
materializing the parent. This change makes sure to precisely call
materializeInParent whenever the state transitions to materialized.
Reviewed By: wez
Differential Revision: D6389794
fbshipit-source-id: 1e740e133a83d5090a6b9801154b7eaeccb07f22
Summary:
To make the materialization code paths a bit clearer, this decouples
materialization from a blob and truncation. It also caches opened
files if openCount > 0 in both the truncation and materialization from
blob paths.
Reviewed By: wez
Differential Revision: D6388318
fbshipit-source-id: c95a85f5bdaa405130f2f7260143592cdc90d45e
Summary:
This corrects a bug where the in-memory timestamp for new files would
be set to the last checkout time instead of when the file was created.
Reviewed By: simpkins
Differential Revision: D6366189
fbshipit-source-id: c5fa8c779726d3a75c2d57b2a161293297eb9271
