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
Summary:
In some workloads we're seeing folks run out of file descriptors.
We forgot that we'd taken out the code that closes the underlying fds.
This diff takes a run at adding a simple counter of the open file handle
objects that is incremented when they are constructed and decremented
when they are destroyed.
When the count falls to zero we release the file handle.
Note that we unconditionally open files when we first load the inodes
from the overlay. I tried to defer that open attempt and it broke
the timestamp overlay test. I think we can revisit that aspect in
a follow on diff; for now we should be more resilient to transiently
opened files from things like ripgrep or similar.
Reviewed By: simpkins
Differential Revision: D6097090
fbshipit-source-id: 9a48220002e760fb1ffb8d7e2a68fa7036558b78
Summary:
In preparation for bringing D6097090 across the finish line, we need a
more explicit mechanism for determining which state the inode is in.
The issue is that whether an inode is materialized was checked in a
couple ways: the nonexistence of the hash field or the definedness of
the file field. This diff introduces an explicit enum indicating the
state.
Reviewed By: simpkins
Differential Revision: D6325955
fbshipit-source-id: 3682a4ebc9330193baadbb33a4dd9845f26e59a6
Summary:
This is preparation for a change I'm preparing that makes the state
transitions in FileInode clearer and safer.
Reviewed By: wez
Differential Revision: D6304272
fbshipit-source-id: 493ee80517443432f790abf9806000eecb03651c
Summary:
With the subsequent diff that enables multiple concurrent
hg importers, I was seeing this deadlock during rebase; each of
the worker threads was being blocked until it saturated the various
thread pools and locked up the server.
This removes the blocking call and replaces it with a SharedPromise
to allow multiple callers to wait for the load.
Note that this changes the semantics of ensureDataLoaded slightly:
previously the blob load was synchronous while the write lock was
held. Now we release the write lock until the load completes.
I've added some post-condition checks to validate that we don't
break any state. I believe that our usage is such that we don't
do anything that might mess things up. Am I missing anything?
Reviewed By: simpkins
Differential Revision: D6264900
fbshipit-source-id: 4aa2870d95f0f0ec48d87299978cb87af99e3969
Summary:
Address some feedback on D6264900 by only storing the sha-1 in the
overlay when it's immediately available. This avoids the possibility
of the FUSE thread getting blocked.
Reviewed By: simpkins
Differential Revision: D6292175
fbshipit-source-id: 06c2372724e58c485d9a302dde36c34885109acf
Summary: Make this function match our C++ guidelines.
Reviewed By: wez
Differential Revision: D6288591
fbshipit-source-id: 1a4f52a8c1e0497df938533fe29da10264eb1ccf
Summary: The comments and the code did not agree on what fields were valid when, so I corrected them and added runtime enforcement.
Reviewed By: wez
Differential Revision: D6260555
fbshipit-source-id: 13e35e22c44b9a5b4cb30cdfd48e2bf7b0e116d3
Summary: Continue moving the asynchrony of loading closer to the state access so, in a future world, we still behave if unloading occurs during access.
Reviewed By: simpkins
Differential Revision: D6238523
fbshipit-source-id: 722fe5bba90b55204d50393314d225f42680409b
Summary: To fix up the invariants in FileInode's API, we intend to remove ensureDataLoaded() and materializeForWrite(). But first I'm going to see if there is any pain caused by removing their calls.
Reviewed By: simpkins
Differential Revision: D6234049
fbshipit-source-id: c39c6d018d164b32903414d3750b875a897af210
Summary:
Per discussion with bolinfest, this brings Eden in line with clang-format.
This diff was generated with `find . \( -iname '*.cpp' -o -iname '*.h' \) -exec bash -c "yes | arc lint {}" \;`
Reviewed By: bolinfest
Differential Revision: D6232695
fbshipit-source-id: d54942bf1c69b5b0dcd4df629f1f2d5538c9e28c
Summary:
This is a mechanical and dumb move of the code from MountPoint
and into the EdenMount class.
Of note, it doesn't merge together the two different state/status fields
into a unified thing; that will be tackled in a follow on diff.
Reviewed By: bolinfest
Differential Revision: D5778212
fbshipit-source-id: 6e91a90a5cc760429d87a475ec12f81b93f87be0
Summary:
We're not doing anything with this today. It's not
clear whether we should be doing sanity checks (eg: block attempts
to write to a handle that was opened only for reading) or whether
the kernel is going to do that for us, so I've broken this out
as a separate diff from the removal of FileData.
Reviewed By: bolinfest
Differential Revision: D5723064
fbshipit-source-id: b73452dfb4edf88b57fef1ad604bb2bde93bacc1
Summary: These don't exist any more, so remove them
Reviewed By: bolinfest
Differential Revision: D5722861
fbshipit-source-id: 7db112dfab1dfdcf517452b314bd912ec8760bd1
Summary: Removed creation time from `FileInode::state` which was used for getting timestamps of files that are not materialized.Now that we added timestamps to file inodes and tree inodes we no longer need this.
Reviewed By: wez
Differential Revision: D5552761
fbshipit-source-id: 6013b1f694045e08ada7bd64114c4f2e52848fef
Summary:
updating atime,ctime,mtime of FileInode on read, write and setattr system calls.
modified `FileInode::stat` function to return accurate inmemory timestamps.
Reviewed By: simpkins
Differential Revision: D5552666
fbshipit-source-id: 86d446f72908663f8db509b7b789d9f35d17df3a
Summary: Removed `FileInode::setattr` from `FileInode` and added a helper function `setInodeAttr` to perform FileInode or TreeInode specific setattr operations in `InodeBase::setattr`. This diff contains implementation of setattr for FileInode i.e for files, will add setattr implementation for directories in another diff.
Reviewed By: simpkins
Differential Revision: D5544968
fbshipit-source-id: 089491d07a603e111966987ef390b6e597aba28c
Summary:
1. Added a new structure `InodeBase::InodeTimestamps` to wrap atime,ctime,mtime together. This new structure helps in avoiding usage of `struct stat` for timestamps.
2. Modified function `Overlay::openFile` ,`Overlay::updateTimestampToHeader`, `Overlay::deserializeOverlayDir`, `Overlay::parseHeader` to use this new structure for timestamps instead of `struct stat`. Also, modified code in places where this change is being affected.
3. Added new helper methods `FileInode::setattrTimes` and `TreeInode::setattrTimes` to set timestamps in FileInode and TreeInode during setattr. Implementation of setattr for FileInode and TreeInode is in the diffs stacked above this diff.
4. Replaced atime, ctime, mtime in `FileInode::State`, `TreeInode::Dir` to `FileInode::State::timeStamps` and `TreeInode::State::timeStamps`. Made other necessary changes to support this change.
Reviewed By: simpkins
Differential Revision: D5596854
fbshipit-source-id: 2786b7b695508a62fdf8f7829f1ce76054b61c52
Summary: Currently we have two functions `FileInode::setattr` and `FileInode::setAttr` which are used to set given attributes to a `FileInode`. Merged both the functions in to one function called `FileInode::setattr` and removed `FileInode::setAttr`.
Reviewed By: wez
Differential Revision: D5538490
fbshipit-source-id: ec241fad25d6e4694865e5fc3c0a3500e4838bdd
Summary:
Added a new function `InodeBase::updateOverlayHeader` and implemented `FileInode::updateOverlayHeader` and `TreeInode::updateOverlayHeader` to update inmemory timestamps to overlay header when an inode is unreferenced.
Added helper functions in `Overlay` class to read and update timestamps in to the overlay file. Also,modified `Overlay::loadOverlayDir` to read and populate timestamps from overlay header in to treeinode.
Modified constructor of `FileInode::state` to read timestamps from overlay file and to populate inode timestamps.
Added test case to check if time stamps are updated and read correctly on remount.
Fixed a lint warning in TARGETS file
Reviewed By: simpkins
Differential Revision: D5535429
fbshipit-source-id: f6b758f70101c65d316a35101aacc9a3363f7aed
Summary:
In testing, we discovered a case where concurrent Hg operations in EdenFS would
//sometimes// fail with `ECONNREFUSED` when trying to read
`<eden-mount>/.eden/socket`.
This was very confusing, as the standard reasons for `ECONNREFUSED` did not seem
to apply:
- We verified that Eden had not crashed.
- We verified that Eden's UNIX domain socket had a sufficiently large backlog
(1024) such that we should not be at risk of exhausting it with two simultaneous
Hg commands.
It turned out that there was a race condition in our `readlink()` command, which
could cause `<eden-mount>/.eden/socket` to resolve to the wrong path. Failing to
connect to this path manifested itself as an `ECONNREFUSED` error.
It turned out that `readlink()` used `FileInode::readAll()`, which was
performing an `lseek()` followed by a `read()` on a file descriptor while the
descriptor was protected by a //read lock// instead of a //write lock//. Because
the `lseek()` causes a state change, it needs to be protected by a write lock.
Changing the type of the lock fixed the issue.
The only other caller of `FileInode::readAll()` appears to be in
`TreeInode::loadGitIgnoreThenDiff()`, so presumably this fixes a possible race
condition there, as well.
Reviewed By: simpkins
Differential Revision: D5533001
fbshipit-source-id: 86cf84c45463b2ae194d6f46909ea67c0f71d065
Summary: Added atime,ctime,mtime for tracking timestamps for directories inmemory and initialized them to the last checkout time during the creation of TreeInode.
Reviewed By: bolinfest
Differential Revision: D5440950
fbshipit-source-id: 639cf1ce6722f80dde35d33849aa712aa30301a8
Summary:
Added a new data member lastCheckoutTime to EdenMount class to store the time when checkout operation is performed. Also added a method to get the last checkout time which internally returns the lastCheckoutTime in EdenMount class.
Added new fields atime,mtime,ctime in FileInode::State structure to keep track of timestamps in memory. Initialzed timestamps in FileInode::State constructor by calling getLastCheckOutTime from EdenMount class.
Still have to add timestamp tracking for directories and have to initialize them with lastCheckout time.This probably will be done in a seperate diff.
Reviewed By: bolinfest
Differential Revision: D5437682
fbshipit-source-id: e3d6b1bc0c2192538dd3b0d9a6017ceb3ca0843d
Summary:
Moved all the member functions from `FileData` class to `FileInode` class
and made `FileInode` methods independent of shared `FileData` object.
Removed `FileData.h` and `FileData.cpp` files as they are not needed anymore.
Modified functions `FileInode::getSHA1()` and `FileInode::isSameAsFast` and
modified few testcases which are currently using `FileData` class and made
sure that all the test cases are passing.
Reviewed By: bolinfest
Differential Revision: D5430128
fbshipit-source-id: 3e8e6c490e92e4e602355e4ce39b67c450ec53f8
Summary:
1.Refactored FileInode and FileData code.
2.Moved some data members of file data into struct State inside file inode
3.Refactoring code such that FileData and FileInode classes are eventually moved to FileInode class.
Reviewed By: simpkins
Differential Revision: D5414427
fbshipit-source-id: cf24721a65541ddfdec7ead4a035de4da3fd5bb5
Summary:
[Folly] Remove `construct_in_place`.
It is an alias for `in_place`, and unnecessary.
Reviewed By: Orvid
Differential Revision: D5362354
fbshipit-source-id: 39ce07bad87185e506fe7d780789b78060008182
Summary:
This replaces the shouldFileBeDeletedByHgRemove() function with a much simpler
and safer check.
It turns out this code was more complicated than it needed to be: it looked up
the inode in question and confirmed that it still existed, even though it's
calling function had already previously looked up the inode object.
I believe this deletes the last place in the code that still assumes that an
InodeBase object must be loaded if the inode has previously been materialized.
This also removes one of the last remaining places that directly holds the
TreeInode contents_ lock outside of TreeInode.cpp.
Reviewed By: bolinfest
Differential Revision: D4968834
fbshipit-source-id: a37bc0d7889eb81cca5b045cbc82732a53ef53d4
Summary:
This change makes it so that all of the C++ code related to the edenfs daemon
is now contained in the eden/fs subdirectory.
Reviewed By: bolinfest, wez
Differential Revision: D4889053
fbshipit-source-id: d0bd4774cc0bdb5d1d6b6f47d716ecae52391f37
Summary:
This diff fixes FileData::stat() so that we report reasonable timestamp values
on non-materialized files, rather than always leaving them as 0. We set the
timestamps to the time that we created the FileInode. This ensures that
timestamps are updated correctly when files are modified by a checkout
operation.
Note that for materialized files the code reports the timestamp of the overlay
file. This diff does not modify that behavior. However, this behavior is
incorrect, as the overlay file timestamps are not updated by a FUSE client
opening, modifying, then closing a file (since we keep the underyling overlay
file handle open, and don't close it).
In the future we'll need to implement our own tracking of atime, mtime, and
ctime values. We should probably store these in a header inside the overlay
file. For now, this diff is a stop-gap measure that ensures we at least update
non-materialized file timestamps correctly on checkouts.
Reviewed By: bolinfest
Differential Revision: D4765632
fbshipit-source-id: 478da6441e213cdfe830f1c5129212182ce4eeb0
Summary:
this is the bare minimum to support creating unix domain sockets.
We only support using mknod to create a unix socket; other uses will yield an error.
I've added an rdev field as a sibling of the existing mode field that we track,
as that is the additional parameter that we need to track as part of the
special file node.
Special file nodes are tracked in the overlay as empty files.
Reviewed By: bolinfest
Differential Revision: D4774099
fbshipit-source-id: 0824b7e509063faa8bede7aff82a7c51930c4f83
Summary:
This begins adding an EdenMount::diff() method, which walks through the inode
tree and reports differences from the current source control tree state.
My intent is to eventually update the Dirstate code to use this diff()
function, and to remove the existing getModifiedDirectories() logic. The
getModifiedDirectories() code is currently incorrect, as it reports
materialized directories, which is not the same as directories that are
modified from the current source control state.
The ignore processing logic is not currently implemented in EdenMount::diff(),
but it should be relatively straightforward to add in a subsequent diff.
Reviewed By: bolinfest
Differential Revision: D4726048
fbshipit-source-id: ad0bb3b5d72bb3830f60fc2b2e56a81217c35353
Summary:
Refactor FileInode and FileData so that they no longer store the
TreeInode::Entry that refers to this file. The TreeInode::Entry is owned by
the parent TreeInode and is not safe to access without holding the TreeInode
contents_ lock, which the FileInode code never did. Additionally, once a
FileInode is unlocked the TreeInode::Entry is destroyed. Previously the
FileInode code would retain its pointer to this now invalid data, and could
still dereference it.
In the future I think it might be worth refactoring the FileInode/FileData
split a bit more, but this at least addresses the issues around invalid
accesses to entry_.
Reviewed By: bolinfest
Differential Revision: D4688058
fbshipit-source-id: 17d5d0c4e756afc6e3c4bb279cb1bb5231f3134f
Summary:
This changes a couple aspects of the materialization process:
- Write out child files in the overlay before their parent. If we throw an
exception or crash partway through this process, it is better to have the
child not marked materialized in its parent, rather than to have the parent
indicate that the child is materialized but to not actually have any overlay
data for the child. In the former case we will still load the correct child
data the next time we need to, but in the latter case we would fail to load
the child correctly.
- Hold the rename lock while materializing our parent directories, to ensure
that we materialize the correct parent, and it cannot change while we are
trying to perform the materialization.
- Make the parent responsible for modifying the child's TreeInode::Entry. This
data is owned by the parent and is protected by the parent's contents_ lock.
The child really shouldn't ever be directly accessing this data without
holding the parent's contents_ lock.
Reviewed By: bolinfest
Differential Revision: D4688057
fbshipit-source-id: 2662f79cb7d7febb086f4e0888a3d96a580c4bfa
Summary:
Add a method to reset the current commit without changing the working
directory state, similar to "hg reset" and "git reset --soft".
This also adds a new EXPECT_FILE_INODE() check for use in the unit tests,
and FileInode::getPermissions() and FileData::readFull() methods to support
this check.
Reviewed By: wez
Differential Revision: D4641476
fbshipit-source-id: 1e516774fe8e292a8d82cc2c354619374a3abe37
Summary:
Previously, we would only allow you to consume a symlink that was
checked into a revision, but not allow you to create a new one.
This diff adds support for making new symlinks.
It turns out that the code that we had for readlink had an unused code path for
reading an actual symlink out of the overlay. My first pass at this diff tried
to make the materialization code generate such a symlink, but it was breakin
some other assumptions in the rest of the code.
This version of the code just stores the symlink target in the file contents.
This means that we can simplify readlink to just calling `readAll` on the
underlying file data, and that will load the contents out of the storage layer
if the file wasn't materialized. This helps simplify things a bit more.
Reviewed By: bolinfest, simpkins
Differential Revision: D4604016
fbshipit-source-id: 3138d0f9880639d2bbeaf4bb03bef3f021c3ecb3
Summary:
This is the initial code for implementing checkout.
This isn't quite 100% implemented yet, but I think it's worth checking in this
code as-is, and getting the remaining functionality done in separate diffs.
In particular, a few operations aren't implemented:
- Removing a directory that was deleted in the new revision
- Replacing a directory that was replaced with a file or symlink in the new
revision
- When doing a forced update, replacing a file or directory that did not exist
in the old revision, but that was created locally in the working directory,
and also exists in the new revision.
Reviewed By: wez
Differential Revision: D4538516
fbshipit-source-id: 5bb4889b02f23ab2048fcae2c8b7614340181aa6
Summary:
Refactor the Overlay code to store data using inode numbers rather than the
affected file's path in the repository. This simplifies the TreeInode code a
bit, as we no longer have to rename overlay files to stay in sync with the file
paths. This also eliminates some crashes when trying to update overlay files
for inodes that have been unlinked (and hence no longer have a path). This
also includes a few fixes to avoid writing journal entries for unlinked files
too. Additionally this contains a few fixes to how mode bits are stored in the
overlay, and fixes a bug where create() was ignoring the mode argument.
Reviewed By: wez
Differential Revision: D4517578
fbshipit-source-id: c1e31497dcf62c322b0deff72b0a02675b0509ab
Summary:
Update copyright statements to "2016-present". This makes our updated lint
rules happy and complies with the recommended license header statement.
Reviewed By: wez, bolinfest
Differential Revision: D4433594
fbshipit-source-id: e9ecb1c1fc66e4ec49c1f046c6a98d425b13bc27
Summary:
This defines our own custom smart pointer type for Inode objects. This will
provide us with more control over Inode lifetime, allowing us to decide if we
want to unload them immediately when they become unreferenced, or keep them
around for a while.
This will also allow us to fix some memory management issues around EdenMount
destruction. Currently we destroy the EdenMount immediately when it is
unmounted. This can cause issues if other parts of the code are still holding
references to Inode objects from this EdenMount. Our custom InodePtr class
will also allow us to delay destroying the EdenMount until all of its Inodes
have been destroyed.
This diff adds the new pointer types and updates the code to use them, but does
not actually implement destroying unreferenced inodes yet. The logic for that
has proven to be slightly subtle; I will split it out into its own separate
diff.
Reviewed By: wez
Differential Revision: D4351072
fbshipit-source-id: 7a9d81cbd226c9662a79a2f2ceda82fe2651f312
Summary:
This member variable was not updated properly when files were renamed.
InodeBase now tracks our parent properly, so we don't need our own copy.
This does still call getParentBuggy() (which does not perform proper locking)
in a couple places for performing overlay operations. We'll need to fix this
later when addressing other overlay concurrency handling issues.
Reviewed By: bolinfest
Differential Revision: D4348481
fbshipit-source-id: 19c1ffced6f63e1ff041d0bab2363fecdb93d5a3
Summary:
Have FileData objects store a pointer to the FileInode that owns them, rather
than just to the EdenMount.
FileData objects still store a direct pointer to their FileInode's mutex_ and
entry_. It's potentially worth just accessing these through inode_ in the
future.
Reviewed By: bolinfest
Differential Revision: D4348103
fbshipit-source-id: 1f8497979bfc89c6a192ca0195209335db0d911c
Summary:
This updates all of the eden code to use the new InodeMap class. This replaces
the InodeNameManager class and the unordered_map previously stored in the
EdenDispatcher.
Reviewed By: bolinfest
Differential Revision: D4325750
fbshipit-source-id: d80ae7581ba79ca2b63155e184995a3e83e85dc1
Summary:
InodeError is a subclass of std::system_error that accepts an InodePtr to the
inode that it refers to. This makes it easier to construct error objects that
retain information about the inode they refer to.
InodeError also avoids computing the inode path until the error message is
actually needed. This should make it less expensive in cases where errors are
thrown and handled internally without ever using the human-readable error
message. It is possible that the file may have been renamed or unlinked by the
time the error message is computed. However, this race condition might still
exist even if we computed the path at the time when the error is constructed.
getLogPath() will construct a usable human-readable string even if the file has
been unlinked.
Reviewed By: wez
Differential Revision: D4325043
fbshipit-source-id: c9683a80b022f281ca4583a9b7f73b15277335bb
Summary:
Hash objects are small enough (20 bytes) that it isn't worth allocating them on
the heap. This updates LocalStore::getSha1ForBlob() to return a
folly::Optional<Hash>, and ObjectStore::getSha1ForBlob() to return a plain
Hash.
Reviewed By: bolinfest
Differential Revision: D4298162
fbshipit-source-id: 9cf54f2997ba8c3b2346db315a2aca41e580b078
Summary:
Define InodePtr, TreeInodePtr, and FileInodePtr as aliases for std::shared_ptr
of the underlying inode type. This also updates all of the code to use these
new type names.
This will make it easier swap out std::shared_ptr with a custom pointer type in
the future. (I believe we will need a custom type in the future so that we
can have more precise control of the reference counting so we can load and
unload Inode objects on demand. std::shared_ptr::unique() doesn't quite
provide the flexibility we need, and is also being deprecated in C++17.)
Reviewed By: bolinfest
Differential Revision: D4297791
fbshipit-source-id: 1080945649290e676f62689592159f1166159b20
Summary:
This updates the InodeBase code to track its location in the filesystem.
Since we do not support hard links, each inode has a single path where it
exists.
Tracking this data allows us to implement getPath() as a method of InodeBase.
This code is not really complete yet, but it seems worth getting the current
code in as-is. The location data is not updated properly on unlinks or
renames, but it looks like the existing InodeNameManager code does not get
updated either. I am working on some additional refactoring of inode object
management, and it will be easier to come back and fix the unlink and rename
handling after this refactoring is further along.
Reviewed By: bolinfest
Differential Revision: D4297591
fbshipit-source-id: 82ceb326e4f9c376f627b1d8f49bb7db3cfc2b0b
Summary:
Move the InodeBase class from the lower-level fusell code up to the
eden/fs/inodes layer, now that everything else that uses it is in
eden/fs/inodes.
I plan to start changing the ownership model of inode objects a bit, and this
will allow the InodeBase class to interact with EdenDispatcher and other
classes in eden/fs/inodes.
Reviewed By: bolinfest
Differential Revision: D4283392
fbshipit-source-id: 9e1d6fb81dc223f905847cbe8d165a40ad0aca4d
Summary:
Now that the EdenDispatcher class has been moved into eden/fs, we no longer
need the distinction between TreeInode and fusell::DirInode, and FileInode and
fusell::FileInode.
This diff deletes the fusell versions of these classes, and updates all of the
code to always directly use TreeInode and FileInode. This allows us to get rid
of the remaining dynamic_casts between these pairs of classes.
Reviewed By: bolinfest
Differential Revision: D4257165
fbshipit-source-id: e2b6f328b9605ca0e2882f5cf7a3983fb4470cdf
Summary:
Rename TreeEntryFileInode to FileInode, and TreeEntryFileHandle to FileHandle.
These class names were long and awkward.
It's slightly unfortunate that we now have classes named both
eden::fuse::FileInode and eden::fuse::fusell::FileInode, but I don't believe
this should cause any major problems. If we want to eliminate these name
collisions in the future I would advocate for renaming the fusell versions to
something like "FileInodeIface".
Reviewed By: bolinfest
Differential Revision: D4217909
fbshipit-source-id: 899672a318d7ae39595f2c18e171f8fd6cebedc6
