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:
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:
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: This gets rid of those pesky #ifdefs in FileInode.cpp and TreeInode.cpp.
Reviewed By: wez
Differential Revision: D7735914
fbshipit-source-id: d810461984e21f72670f43ca2d1b4f5aacbf376e
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:
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:
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: Each Overlay object should be owned only by its EdenMount.
Reviewed By: chadaustin
Differential Revision: D7543294
fbshipit-source-id: 6db40fea31ce298c61d047cba9165887e32926a1
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:
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:
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:
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:
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:
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:
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:
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:
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:
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:
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: 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 `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:
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:
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:
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: Minor stylistic changes that were done during constification but factored out.
Reviewed By: chadaustin
Differential Revision: D6774976
fbshipit-source-id: d18cd339153cf16ff69be0de5f3eb019a4baa1a0
Summary:
This implements a TODO/FATAL that is important for
graceful restarts to be useful in my "acid test" scenario,
which is to perform a graceful restart while buck build is
running.
Reviewed By: simpkins
Differential Revision: D6700189
fbshipit-source-id: dec1b818ebc9e907841bc127ee08c953b59d6487
