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
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:
Update the logic for dry-run checkouts to exit early whenever we hit a
directory that is unmodified from the original source control state. In this
case we can skip this entire subdirectory since we know there cannot be any
conflicts.
This should make dry-run checkout operations much faster in cases where there
are lots of changes between the source and destination source control trees,
but the working directory state has relatively few modified files.
Reviewed By: bolinfest
Differential Revision: D6605282
fbshipit-source-id: b5423749f3d47b10ed8d599ffaa0667c72fbaec2
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:
Added to Eden capability to incorporate default user and general system level gitignore files.
NOTE: Work in progress, sending the review out to calibrate/ensure I am on right track.
Reviewed By: simpkins
Differential Revision: D6482863
fbshipit-source-id: 9834ca1a577a9599a1f8cb2243dca4e714866be8
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: Follow-up from D6366189. First use of the new FakeClock!
Reviewed By: simpkins
Differential Revision: D6466209
fbshipit-source-id: 4d4d8a9a83df2bee11149e7a0cbddaaf734d0e04
Summary:
Update TreeInode::computeCheckoutActions() to short circuit on non-materialized
directories that are identical to both the destination and source state. This
speeds up `hg update --clean` operations by allowing us to skip portions of the
tree that do not require changes.
This does not have much effect for update operations without `--clean`: those
already short-circuited the operation in `TreeInode::processCheckoutEntry()`
when the source and destination hashes were the same. That code cannot
short-circuit in the forced update case, since the forced update may have to
revert local modifications.
Reviewed By: bolinfest
Differential Revision: D6455970
fbshipit-source-id: 393acb3272745751a56e06dba0c7505ff2bfad44
Summary:
Previously, we used the Mercurial code `g` when faced with an `UNTRACKED_ADDED`
file conflict, but that was allowing merges to silently succeed that should not
have. This revision changes our logic to use the code `m` for merge, which
unearthed that we were not honoring the user's `update.check` setting properly.
Because we use `update.check=noconflict` internally at Facebook, we changed the
Eden integration tests to default to verifying Hg running with this setting. To
support it properly, we had to port this code from `update.py` in Mercurial to
our own `_determine_actions_for_conflicts()` function:
```
if updatecheck == 'noconflict':
for f, (m, args, msg) in actionbyfile.iteritems():
if m not in ('g', 'k', 'e', 'r', 'pr'):
msg = _("conflicting changes")
hint = _("commit or update --clean to discard changes")
raise error.Abort(msg, hint=hint)
```
However, this introduced an interesting issue where the `checkOutRevision()`
Thrift call from Hg would update the `SNAPSHOT` file on the server, but
`.hg/dirstate` would not get updated with the new parents until the update
completed on the client. With the new call to `raise error.Abort` on the client,
we could get in a state where the `SNAPSHOT` file had the hash of the commit
assuming the update succeeded, but `.hg/dirstate` reflected the reality where it
failed.
To that end, we changed `checkOutRevision()` to take a new parameter,
`checkoutMode`, which can take on one of three values: `NORMAL`, `DRY_RUN`, and
`FORCE`. Now if the user tries to do an ordinary `hg update` with
`update.check=noconflict`, we first do a `DRY_RUN` and examine the potential
conflicts. Only if the conflicts should not block the update do we proceed with
a call to `checkOutRevision()` in `NORMAL` mode.
To make this work, we had to make a number of changes to `CheckoutAction`,
`CheckoutContext`, `EdenMount`, and `TreeInode` to keep track of the
`checkoutMode` and ensure that no changes are made to the working copy when a
`DRY_RUN` is in effect.
One minor issue (for which there is a `TODO`) is that a `DRY_RUN` will not
report any `DIRECTORY_NOT_EMPTY` conflicts that may exist. As `TreeInode` is
implemented today, it is a bit messy to report this type of conflict without
modifying the working copy along the way.
Finally, any `UNTRACKED_ADDED` conflict should cause an update to
abort to match the behavior in stock Mercurial if the user has the following
config setting:
```
[commands]
update.check = noconflict
```
Though the original name for this setting was:
```
[experimental]
updatecheck = noconflict
```
Although I am on Mercurial 4.4.1, the `update.check` setting does not seem to
take effect when I run the integration tests, but the `updatecheck` setting
does, so for now, I set both in `hg_extension_test_base.py` with a `TODO` to
remove `updatecheck` once I can get `update.check` to do its job.
Reviewed By: simpkins
Differential Revision: D6366007
fbshipit-source-id: bb3ecb1270e77d59d7d9e7baa36ada61971bbc49
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:
Per a discussion with wez, let's make it explicit in the comments that the kernel handles
O_EXCL for us.
Reviewed By: simpkins
Differential Revision: D6365157
fbshipit-source-id: 638f67cd89a9450ff084e0ef77c731ef738bf518
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:
This is follow-up to the lock ordering issues in
StreamingSubscriber. The Journal locks are now finer-grained and no
locks are held while the subscribers are invoked. This change
prevents future deadlocks.
Reviewed By: wez
Differential Revision: D6281410
fbshipit-source-id: 797c164395831752f61cc15928b6d4ce4dab1b68
Summary:
Add log messages that will help debug the behavior of `TreeInode::diff()`
I added this since I saw a case where `getScmStatus()` was returning a file as
modified when it did not appear to have mode or contents changes.
Unfortunately we did not have any existing debug log messages for this, and the
problem went away after restarting edenfs, so I wasn't able to track it down.
Having these log messages will help debug this in the future if we run into it
again.
Reviewed By: bolinfest
Differential Revision: D6281905
fbshipit-source-id: 736028d6efe46a387bed6bf98d971685f83da3ec
