Commit Graph

221 Commits

Author SHA1 Message Date
Wez Furlong
aa7a07f209 remove warning about already loaded inodes
Summary:
Since adding prefetch support, we're triggering this code
each time we perform a lookup operation on a dir which means that
the eden logs are made a bit more noisy with this output than
previously.

So, let's remove the warning.

Reviewed By: simpkins

Differential Revision: D5905454

fbshipit-source-id: 0abc9b42284a5224cef908293ab377d8858977ec
2017-10-17 13:07:50 -07:00
Wez Furlong
25a9786ca5 augment JournalDelta with unclean paths on snapshot hash change
Summary:
We were previously generating a simple JournalDelta consisting of
just the from/to snapshot hashes.  This is great from a `!O(repo)` perspective
when recording what changed but makes it difficult for clients downstream
to reason about changes that are not tracked in source control.

This diff adds a concept of `uncleanPaths` to the journal; these are paths
that we think are/were different from the hashes in the journal entry.

Since JournalDelta needs to be able to be merged I've opted for a simple
list of the paths that have a differing status; I'm not including all of
the various dirstate states for this because it is not obvious how to
reconcile the state across successive snapshot change events.

The `uncleanPaths` set is populated with an initial set of different paths as
the first part of the checkout call (prior to changing the hash), and then is
updated after the hash has changed to capture any additional differences.

Care needs to be taken to avoid recursively attempting to grab the parents lock
so I'm replicating just a little bit of the state management glue in the
`performDiff` method.

The Journal was not setting the from/to snapshot hashes when merging deltas.
This manifested in the watchman integration tests; we'd see the null revision
as the `from` and the `to` revision held the `from` revision(!).

On the watchman side we need to ask source control to expand the list of
files that changed when the from/to hashes are different; I've added code
to handle this.  This doesn't do anything smart in the case that the
source control aware queries are in use.  We'll look at that in a following
diff as it isn't strictly eden specific.

`watchman clock` was returning a basically empty clock unconditionally,
which meant that most since queries would report everything since the start
of time.  This is most likely contributing to poor Buck performance, although
I have not investigated the performance aspect of this.  It manifested itself
in the watchman integration tests.

Reviewed By: simpkins

Differential Revision: D5896494

fbshipit-source-id: a88be6448862781a1d8f5e15285ca07b4240593a
2017-10-16 22:46:54 -07:00
Michael Bolin
9e8e24d7df Create fix and test for hg merge.
Summary:
Running Mercurial's own integration tests revealed that we had a bug here:
https://www.mercurial-scm.org/repo/hg/file/tip/tests/test-histedit-arguments.t

Somewhat unsurprisingly, it was time to finally address a longstanding `TODO`
in `Dirstate.cpp`. The issue was that, after running `hg merge --tool :local`,
`hg status` was not including a merged file in the list of modified files. Because
the information from `hg status` is used to create a commit context, that meant
that when a commit was made after running `hg merge`, the commit did not
include the merged file in the list of files for the commit, which differs from
Mercurial's behavior.

Most of the implementation of `hg status` on the Eden side is done by
`EdenMount.diff()`. However, in this case, `diff()` does not categorize the
merged file by invoking one of the methods of `InodeDiffCallback` because
as far as `EdenMount` is concerned, the file has not changed because `EdenMount`
is unaware of the `Dirstate`. We already have some existing cases where we have
to do some post-processing on the result of `EdenMount.diff()` using information
in the `Dirstate` (e.g., files that are marked for addition or removal), so the fix was
to add a check for the case when the file is flagged as "needs merging" and
then including it as modified in the `hg status` output, as appropriate.

Reviewed By: wez

Differential Revision: D6005603

fbshipit-source-id: 7d4dd80e1a2e9f4b98243da80989e0e9119a566d
2017-10-09 11:55:34 -07:00
Yedidya Feldblum
eb45944ea0 CodeMod: Change #include's of wangle/concurrent/GlobalExecutor.h to use folly
Summary:
CodeMod: Change `#include`'s of `wangle/concurrent/GlobalExecutor.h` to use `folly`.

The file in `wangle/` is just a shim around the same file in `folly/executors/GlobalExecutor.h`. Just codemod all the `#include` sites.

Reviewed By: Orvid

Differential Revision: D5981467

fbshipit-source-id: ad7f0dce959e2760d3977b04925945e0447abc1d
2017-10-05 13:07:45 -07:00
Jeremy Fitzhardinge
fe4a7b6765 eden: enable rust Thrift generation
Reviewed By: wez

Differential Revision: D5909038

fbshipit-source-id: a4bdeac30a17682b2a92e182ab53d0413ad54256
2017-10-03 00:54:17 -07:00
Wez Furlong
364398cf94 ObjectStore now returns shared_ptr<const>
Summary:
Originally I thought this would help move towards removing a
`future.get()` call from FileInode, but it turned out to not make a difference
to that code.

It does make it a bit less of a chore to deal with the Journal related diff
callbacks added in D5896494 though, and is a move towards a future where we
could potentially return cached and shared instances of these objects.

This diff is a mechanical change to alter the return type so that we can share
instances returned from the object store interface.  It doesn't change any
functionality.

Reviewed By: simpkins

Differential Revision: D5919268

fbshipit-source-id: efe4b3af74e80cf1df20e81b4386450b72fa2c94
2017-09-29 16:54:05 -07:00
Chad Austin
3fb5680152 Rename ConflictType::MODIFIED to ConflictType::MODIFIED_MODIFIED
Summary: Per wez, this makes the MODIFIED case consistent with the other conflict types (e.g. local_remote).  Side benefit of avoiding some naming conflicts in the Haskell/Rust thrift tooling.

Reviewed By: wez

Differential Revision: D5882327

fbshipit-source-id: 3ec68c44d8c8a5c5675f1ced3842d29376d46fe2
2017-09-21 16:54:37 -07:00
Chad Austin
24c0fe9e8f prefetch inode contents when they're looked up
Reviewed By: wez

Differential Revision: D5817915

fbshipit-source-id: 3820f635cc6251ae5e13a4e214ba66df25ab6687
2017-09-19 11:10:11 -07:00
Wez Furlong
fab0c4071c centralize the post-mount functionality
Summary:
This moves the bind mounting and post-clone script running
functionality to methods of the EdenMount class and makes the whole
mount flow return a `Future<>`.  The higher level goal is to make it
easier to see where and how we want to tweak this flow to support
graceful restart.

This is mostly straight forward but care is required to avoid deadlocks; there
are two scenarios:

  # We fulfill the fuse start promise in the context of the fuse thread that is
  handling the fuse initialization packet before it has signalled to the kernel
  that it has come up.  This can be solved by using `via(mainEventBase_)`, but...
  # When remounting all the mounts on startup, we're running in the
  `mainEventBase_` thread so the simplistic solution to 1. would cause us to
  deadlock on startup (visible in the remount integration tests).

So to avoid this, we shunt the completion of the future via a CPU pool.

Also worth noting: the way we were setting up the global CPU pool with
wangle wasn't correct; it takes a weak reference to the pool which was
then getting destroyed when our prepare method returned.  It just happened
to work for us in the facebook specific build because something else was
setting up a different CPU executor.

I've reconciled this by just setting up a thread pool of our own and
using that explicitly.

Reviewed By: bolinfest

Differential Revision: D5798659

fbshipit-source-id: f1c48730f283f6962f6cd706c02d82ea2952e369
2017-09-13 08:42:21 -07:00
Michael Bolin
e837848da5 Introduce a special NoValueForKeyError for hgGetDirstateTuple() and hgCopyMapGet().
Summary:
Previously, we were generating a bit of disconcerting noise in our logs when
requesting a non-existent key in the dirstate or its copy map. We were also
susceptible to a logical error in the Eden side being silently translated to
a `KeyError` on the Python side.

Now we make things more explicit by converting a `std::out_of_range` on the C++
side to an explicit `NoValueForKeyError` that is defined in `eden.thrift`.
Now the Python side catches a `NoValueForKeyError` explicitly and converts it
into a `KeyError`. Other types of exceptions should pass through rather than be
swallowed.

This also updates the log messages to communicate when a there is no value for a
key. The messaging is improved so that it no longer appears to be a logical
error.

Reviewed By: wez

Differential Revision: D5800833

fbshipit-source-id: c44f2caf04622475d218593037cc6616bbb1c701
2017-09-11 10:52:09 -07:00
Michael Bolin
1c695e6dc6 Fixed a bug in how we update the dirstate upon a snapshot change.
Summary:
Previously, we were clearing entries in `hgDirstateTuples` for which:

```
mergeState == NotApplicable
```

but we should have been checking for:

```
mergeState == NotApplicable AND status == Normal
```

The previous logic was causing us to erroneously clear entries in a state like:

```
mergeState == NotApplicable AND status == MarkedForRemoval
```

This bug manifested itself when grafting a change that removed a file.
The file was removed from disk, but Eden did not know that it had been
`MarkedForRemoval`, so it would report the removed file as "missing" in
`hg status`.

Reviewed By: wez

Differential Revision: D5797270

fbshipit-source-id: 29740dfaa8102db868b95e932716773787f317ac
2017-09-08 19:25:34 -07:00
Wez Furlong
d910ab6594 unify state and fuseStatus in EdenMount
Summary:
After performing the dumb merge of EdenMount and MountPoint in
the prior commit, this one tidies up the state tracking and the interface
by which clients of the object can be notified of state changes.

I've introduced two Promises; the first of these can be used to wait
for the fuse mount to come up or error out.  It logically replaces
the cond wait in the `start` method and is exposed to the caller
as a Future, allowing them to wait and react to the outcome.

The second of the promises is associated with the fuse thread pool
winding down.  The attached future can be extracted and used by the
client of the EdenMount class.  This future yields the fuse device
descriptor which we can then choose to pass on during graceful
restart or simply close.  In the current integration, since we ignore
the result of that future, the handle is implicitly closed.

These promises allow us to remove the reference cycle that we had with the
`onStop` function and to potentially make the mount/unmount sequence more
async.

Reviewed By: bolinfest

Differential Revision: D5778214

fbshipit-source-id: 00b293009b7251ddd8bfb10795a115188e97aa3a
2017-09-08 19:25:34 -07:00
Wez Furlong
3da68e5adc dumb merge of MountPoint into EdenMount
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
2017-09-08 19:25:34 -07:00
Wez Furlong
e72a4cc187 Dispatcher no longer needs to know about MountPoint
Summary:
This is leading up to folding the MountPoint code into
the EdenMount class.

There's still a mention of the MountPoint in Dispatcher.h; that is
being dealt with in the following diff.

Reviewed By: bolinfest

Differential Revision: D5778215

fbshipit-source-id: 996640b3773988a4738ad55bb13de45e1ffe1880
2017-09-08 19:25:34 -07:00
Wez Furlong
467c417ccb re-organize the fuse Channel and Session code
Summary:
The higher level goal is to make it easier to deal
with the graceful restart scenario.

This diff removes the SessionDeleter class and effectively renames
the Channel class to FuseChannel.  The FuseChannel represents
the channel to the kernel; it can be constructed from a fuse
device descriptor that has been obtained either from the privhelper
at mount time, or from the graceful restart procedure.  Importantly
for graceful restart, it is possible to move the fuse device
descriptor out of the FuseChannel so that it can be transferred
to a new eden process.

The graceful restart procedure requires a bit more control over
the lifecycle of the fuse event loop so this diff also takes over
managing the thread pool for the worker threads.  The threads
are owned by the MountPoint class which continues to be responsible
for starting and stopping the fuse session and notifying EdenServer
when it has finished.  A nice side effect of this change is that
we can remove a couple of inelegant aspects of the integration;
the stack size env var stuff and the redundant extra thread
to wait for the loop to finish.

I opted to expose the dispatcher ops struct via an `extern` to
simplify the code in the MountPoint class and avoid adding special
interfaces for passing the ops around; they're constant anyway
so this doesn't feel especially egregious.

Reviewed By: bolinfest

Differential Revision: D5751521

fbshipit-source-id: 5ba4fff48f3efb31a809adfc7787555711f649c9
2017-09-08 19:25:34 -07:00
Wez Furlong
f30850b8dc avoid loading blob/tree metadata during hg status
Summary:
This partially fixes up a perf problem when performing status when a large
number of inodes have been loaded but not materialized (eg: by `find /edenfs
-ls`).

For the FileInode case we'd end up requesting the SHA1 from the store
twice in parallel only to compare it and decide that the file has not
been changed(!)

The remediation is to cut this code over to calling `FileInode::isSameAs` so that
we can short-circuit some of this work.  In addition, we can avoid loading
subtrees if we haven't materialized them and the hash matches up.

Reviewed By: simpkins

Differential Revision: D5783044

fbshipit-source-id: f40da3fadfcf8d9e19221d41e3a5a980454717db
2017-09-07 14:50:42 -07:00
Michael Bolin
83b3c38095 Fix for hg split in Eden.
Summary:
Before this change, `hg split` crashed complaining that `node` was a
`changectxwrapper` instead of a 20-byte hash when it was sent as `parent1`
of `WorkingDirectoryParents` in `resetParentCommits()`. Now we use `node()` to
get the hash from the `destctx` that we have already extracted via this line
earlier in `merge_update()`:

    destctx = repo[node]

The change to `eden/hg/eden/__init__.py` eliminated the crash, but was
not sufficient on its own to make `hg split` work correctly. There was also a fix
required in `Dirstate.cpp` where the `onSnapshotChanged()` callback was clearing out
entries of both `NotApplicable` and `BothParents` from `hgDirstateTuples`.
It appears that only `NotApplicable` entries should be cleared. (I tried leaving
`NotApplicable` entries in there, but that broke `eden/integration/hg/graft_test.py`.)

I suspected that the logic to clear out `hgDestToSourceCopyMap` in
`Dirstate::onSnapshotChanged` was also wrong, so I deleted it and all of the
integration tests still pass. Admittedly, we are pretty weak in our test coverage
for use cases that write to the `hgDestToSourceCopyMap`. In general, we should
rely on Mercurial to explicitly remove entries from `hgDestToSourceCopyMap`.
We have a Thrift API, `hgClearDirstate()`, that `eden_dirstate` can use to categorically
clear out `hgDirstateTuples` and `hgDestToSourceCopyMap`, if necessary.

Finally, creating a proper integration test for `hg split` required creating a value for
`HGEDITOR` that could write different commit messages for different commits.
To that end, I added a `create_editor_that_writes_commit_messages()` utility as a
method of `HgExtensionTestBase` and updated its `hg()` method to take `hgeditor`
as an optional parameter.

Reviewed By: wez

Differential Revision: D5758236

fbshipit-source-id: 5cb8bf4207d4e802726cd93108fae4a6d48f45ec
2017-09-06 21:20:45 -07:00
Wez Furlong
9f07485239 add code to serialize FileHandleMap
Summary:
The serialized data for each file handle needs to be enough
to re-construct the handle when we load it into a new process later
on.  We need the inode number, the file handle number that we communicated
to the kernel and a flag to let us know whether it is a file or a dir.

Note that the file handle allocation strategy already accomodates the
idea of migrating to a new process; we don't need to serialize anything
like a next file handle id number.

This doesn't implement instantiating the handles from the loaded state,
it is just the plumbing for saving and loading that state information.

Reviewed By: bolinfest

Differential Revision: D5733079

fbshipit-source-id: 8fb8afb8ae9694d013ce7a4a82c31bc876ed33c9
2017-08-30 19:20:23 -07:00
Wez Furlong
a32c744daf remove FileHandle::openFlags_
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
2017-08-30 19:20:23 -07:00
Wez Furlong
bdecf8b1f3 remove dead FileData references
Summary: These don't exist any more, so remove them

Reviewed By: bolinfest

Differential Revision: D5722861

fbshipit-source-id: 7db112dfab1dfdcf517452b314bd912ec8760bd1
2017-08-30 19:20:23 -07:00
Jyothsna Konisa
8fb37c1ada Diagnostic tool to report Stat information of EdenFs
Summary:
Added new tool to report stat information of EdenFs like fuse counters, Memory counters, latencies, Inode status for all the mount points etc.

eden stat : Prints the general information about eden like list of mount points, loaded unloaded and materialized inodes in each mount point. Also this reports how well periodic unload job is doing by reporting the number of unloaded inodes by periodic job.

eden stat io : Prints how many number of calls made to a system call in Edenfs.

eden stat memory : returns the memory stat for edenfs.

eden stat latency : reports the latencies of system calls in Edenfs.

Reviewed By: bolinfest

Differential Revision: D5660345

fbshipit-source-id: 97a1c2b83a6d8df0cd1b82c4d54b52d7ebd126bd
2017-08-25 12:49:35 -07:00
Braden Watling
ab43c66a8d Add test to verify that eden debug getpath indicates when inodes are unloaded
Summary:
This test was supposed to be a part of D5627411 but it was causing strange behaviour so was brought to a separate diff for further investigation.

After investigating, the test didn't pass because the UnloadedInodeData struct only contained the name of the file, not the path to it. The fix for this was to implement a way to get the relative path of the file even after the inode is unloaded.

Reviewed By: simpkins

Differential Revision: D5646929

fbshipit-source-id: f166398a651e8aea49da7e4474a5ad7fde2eaa4e
2017-08-25 08:34:31 -07:00
Jyothsna Konisa
72b61a5ddc Changes to return unloaded inode count for TreeInode::unloadChildrenNow
Summary:
1.Modified `TreeInode::unloadChildrenNow()` to return number of inodes that have been unloaded.
2.Modified `EdenServiceHandler::unloadInodeForPath()` to return number of inodes that are unloaded.

Reviewed By: simpkins

Differential Revision: D5627539

fbshipit-source-id: 4cdb0433dced6bf101158b9e6f8c35de67d9abbe
2017-08-22 19:50:00 -07:00
Michael Bolin
5cf1692782 Add new Thrift API: hgClearDirstate(mountPoint)
Summary:
When Hg tells the `dirstate` to `clear()`, we should also clear out any data we
have on the server for the `Dirstate`.

As it stands, the way we subclass `dirstate`, it does not appear like `clear()`
should be called, in practice, though one thing that could call it is
`hg debugrebuilddirstate`. It is probably good for us to have an RPC lying
around that we can use to reset the `Dirstate.`

Reviewed By: wez

Differential Revision: D5675298

fbshipit-source-id: 38926cfd93f4f83e4c28910f812a693cb32e423a
2017-08-22 16:50:24 -07:00
Michael Bolin
e050ffcce2 Add new Thrift API: hgDeleteDirstateTuple(mountPoint, relativePath)
Summary:
Previously, we were overloading `hgSetDirstateTuple()` to also make it possible
to delete an entry from the internal `hgDirstateTuples` map. Now we have an
explicit method to do this, which enables us to remove some hacks/TODOs.

Reviewed By: simpkins

Differential Revision: D5665170

fbshipit-source-id: bc0753d4990c8966fd9e6c50b29a954d5023292f
2017-08-18 21:49:59 -07:00
Michael Bolin
5a2246acbf Address a major outstanding TODO in Dirstate::hgGetDirstateTuple.
Summary:
Previously, `Dirstate::hgGetDirstateTuple()` was reporting a status of
`DirstateNonnormalFileStatus::NotTracked` even when the true status was
`Normal`.

Falsely reporting the status has serious consequences when running `hg log` on
an existing, tracked file. Specifically, it causes this `f not in wctx`
condition to be `True` here:

https://phab.mercurial-scm.org/diffusion/HG/browse/default/mercurial/cmdutil.py;da8bdeb1be28b976909a963c89e974264686e2bb$2316

which in turn causes the slow path to be selected:

https://phab.mercurial-scm.org/diffusion/HG/browse/default/mercurial/cmdutil.py;da8bdeb1be28b976909a963c89e974264686e2bb$2320

For large repositories like ours, this can be very, very slow.

There are still some TODOs in the new implementation, but this seems much more
faithful to the true implementation than what we had before.

Reviewed By: quark-zju

Differential Revision: D5655741

fbshipit-source-id: 07b953e23e4d74c480ac2d94dfc6a8df9df4fcbb
2017-08-18 21:49:59 -07:00
Jyothsna Konisa
43f27195b6 Modification of unloadChildrenNow
Summary: Modified `TreeInode::unloadChildrenNow` such that inodes are unloaded whose age is greater than a specific age.

Reviewed By: simpkins

Differential Revision: D5526137

fbshipit-source-id: 91e2364d55e31befedcf43d98c26467e1a472ef9
2017-08-18 14:20:43 -07:00
Adam Simpkins
4917682fc6 add a tag parameter to ThreadLocal<EdenStats>
Summary:
Update all of the code using ThreadLocal<EdenStats> to pass in a non-default
Tag parameter to the ThreadLocal template.

A non-default tag parameter is required to use the accessAllThreads() method on
the ThreadLocal object.  We need to use accessAllThreads() to perform stats
aggregation correctly.  Currently the EdenServer code is only performing
aggregation for stats in the FunctionScheduler.

This diff only updates the ThreadLocal<EdenStats> type, but does not contain
any behavior changes.  I will fix the stats aggregation in a subsequent diff.

Reviewed By: bolinfest

Differential Revision: D5657268

fbshipit-source-id: bc4b6f56324eb8d3052c023fd3f6f64f99b1d4e0
2017-08-18 11:50:56 -07:00
Jyothsna Konisa
916c129655 setting TimeStamps for TreeInode
Summary:
Updated time stamps of TreeInode accurately on mkdir,rmdir,mknode,symlink,create,unlink and readdir.
updated the `TreeInode::getattr` function to return in-memory timestamps.

Reviewed By: simpkins

Differential Revision: D5568183

fbshipit-source-id: c36f7fb767cd4342aab5cc983eea56e37cd2077e
2017-08-14 23:23:23 -07:00
Jyothsna Konisa
5c373f74c1 Removal of creation time from `FileInode::State
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
2017-08-14 23:23:23 -07:00
Jyothsna Konisa
8bcd0f234c Setting TimeStamps for FileInode
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
2017-08-14 23:23:23 -07:00
Jyothsna Konisa
00bce5b020 implementing TreeInode::setInodeAttr
Summary:
Added `TreeInode::setInodeAttr` a helper function used in `InodeBase::setattr`. Also,added `InodeBase::setAtime` ,`InodeBase::setMtime`  and implemented them in `FileInode` and `TreeInode`.
Moved updating timestamp logic to `InodeBase::setattr` from `FileInode::setInodeAttr` and `TreeInode::setInodeAttr`.

Reviewed By: simpkins

Differential Revision: D5545422

fbshipit-source-id: 597cfabb3062166a058cf32776acb50a1bc0c61c
2017-08-14 13:36:37 -07:00
Jyothsna Konisa
7e230ba743 Implementing setattr in InodeBase (FileInode::setattr removal)
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
2017-08-14 13:36:37 -07:00
Wez Furlong
c08890f849 do a better job at reporting "new" in watchman results.
Summary:
We're seeing that this is always set to true for eden,
which is causing buck to run slower than it should.

To make this work correctly, I've augmented our journal data structure
so that it can track create, change and remove events for the various
paths.

I've also plumbed rename events into the journal.

This requires a slightly more complex merge routine, so I've refactored the two
call sites that were merging in slightly different contexts so that they can
now share the same guts of the merge routine.  Perhaps slightly
counterintuitive in the merge code is that we merge a record from the past into
the state for now and this is a bit backwards compared to how people think.

I've expanded the eden integration test to check that we don't mix up
create/change/removes for the same path in a given window.

On the watchman side, we use the presence of the filename in the createdPaths
set as a hint that the file is new.  In that case we will set the watchman
`ctime` (which is not the state ctime but is really the *created clock time*)
to match the current journal position if the file is new, or leave it set
to 0 if the file is not known to be new.  This will cause the `is_new`
flag to be set appropriately by the code in `watchman/query/eval.cpp`;
if the sequence is 0 then it should never be set to true.  Otherwise (when
the file was in the `createPaths` set) it will be set to the current journal
position and this will be seen as newer than the `since` constraint on
the query and cause the file to show as `new`.

Reviewed By: bolinfest

Differential Revision: D5608538

fbshipit-source-id: 8d78f7da05e5e53110108aca220c3a97794f8cc2
2017-08-11 12:57:37 -07:00
Jyothsna Konisa
3f046593a8 Wrapper for TimeStamps & helper function to set timestamps in setattr.
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
2017-08-11 11:36:07 -07:00
Michael Bolin
48f3d2e5b9 Autoformat TreeInode.cpp before making other, unrelated changes.
Reviewed By: simpkins

Differential Revision: D5600811

fbshipit-source-id: e3d0e1da18937e2a3c7ebb77d730622ff62bbced
2017-08-10 14:55:04 -07:00
Jyothsna Konisa
20c62ae2bf Refactoring FileInode::setattr
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
2017-08-04 20:19:20 -07:00
Jyothsna Konisa
6aa6e547d6 Reading and writing timestamps in to overlay files
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
2017-08-04 20:19:20 -07:00
Adam Simpkins
4949aada7f fix EdenServer::unmount() to fully wait for mount point cleanup
Summary:
This fixes EdenServer::unmount() to actually wait for all EdenMount cleanup
to complete, and fixes unmountAll() to return a Future that correctly waits for
all mount points to be cleaned up.

Previously `unmount()` waited for the mount point to be unmounted from the
kernel, but did not wait for EdenMount shutdown to complete.  Previously
EdenMount shutdown was not triggered until the last reference to the
shared_ptr<EdenMount> was released.  This often happened in the FUSE channel
thread that triggered the mountFinished() call--it would still hold a
reference to this pointer, and would not release it until after
mountFinished() returns.  As a result, when the main thread was shutting down,
`main()` would call `unmountAll()`, and then return soon after it completed.
Some FUSE channel threads may still be running at this point, still performing
`EdenMount` shutdown while the main thread was exiting.  This could result in
crashes and deadlocks as shutdown tried to access objects already destroyed by
the main thread.

With this change `EdenMount::shutdown()` is triggered explicitly during
`mountFinished()`, and `unmount()` will not complete until this finishes.
The `EdenMount` object may still exist at this point, and could still be
deleted by the FUSE channel thread, but the deletion now only requires freeing
the memory and does not require accessing other data that may have been cleaned
up by the main thread.

We should still clean up the FUSE channel thread handling in the future, to
make sure these threads are joined before the main thread exits.  However, that
cleanup can wait until a separate diff.  Ideally I would like to move more of
the mount and unmount logic from EdenServer and EdenServiceHandler and put that
code in EdenMount instead.

Reviewed By: bolinfest

Differential Revision: D5541318

fbshipit-source-id: 470332478357a85c314bc40458373cb0f827f62b
2017-08-02 17:07:19 -07:00
Jyothsna Konisa
6d2b510314 Fix in initializing last checkout time in Treeinode.
Summary: Changed the initialization of last checkout time in Tree inode constructor by grabbing a lock on `TreeInode::contents_`

Reviewed By: simpkins

Differential Revision: D5524857

fbshipit-source-id: e86ee7d986ca0c280ba156ba9146d6d1f9fa722e
2017-07-31 20:52:35 -07:00
Michael Bolin
50191d5bbd Fix a race condition in FileInode::readAll().
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
2017-07-31 19:08:09 -07:00
Adam Simpkins
10be70e0a3 minor cleanups to the strace-style logging
Summary:
Make a few minor tweaks to the strace-style logging added in D5464387.

- Call the log categories "eden.strace.<mount_path>" instead of
  "eden/strace<mount_path>".  The folly logging library uses '.' to separate
  nodes in the log category hierarchy, so this puts all of the strace messages
  under the "eden.strace" category, which it itself part of the "eden"
  category.  The "<mount_path>" will contain slashes inside it, but slashes are
  not treated specially in log category names.

- Rename `EdenMount::getLogger()` to `EdenMount::getStraceLogger()` since this
  logger should be used only for strace-style events, and not for general log
  messages for this mount point.

Reviewed By: bolinfest

Differential Revision: D5515245

fbshipit-source-id: 9d833d9fbff47c6a57a7afefeae92755ff0e28b7
2017-07-27 20:20:39 -07:00
Jyothsna Konisa
e4fefa3e69 Adding Timestamps to TreeInode class and intializing timestamps to lastcheckout time
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
2017-07-27 18:25:01 -07:00
Jyothsna Konisa
19df19d994 Adding lastCheckoutTime to EdenMount and initializing timestamps of FileInode with lastCheckoutTime
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
2017-07-27 11:52:31 -07:00
Jyothsna Konisa
20cd12b31a Moving FileData methods to FileInode
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
2017-07-26 23:39:35 -07:00
Adam Simpkins
6f60e9318e fix crashes triggered by the new TreeInode::Entry checks
Summary:
D5483953 added a check to ensure that getMode() can only be called on entries
that do not have a loaded inode.  However, a few places in the code were still
calling getMode() on tree entries without checking if the inode was loaded or
not.

These crashes were caught in the integration tests run by `buck test eden/...`,
but were not caught by sandcastle tests on the original diff since sandcastle
only runs the eden unit tests, and not the integration tests.

All of these locations only needed to check the file type, which is safe to do
even if the inode is loaded, since the file type portion of the mode can never
change on an existing inode.  Only the permissions bits are unsafe to access
once an inode has been loaded (since we need to ask the inode itself for the
latest permissions bits).

I updated these call sites to stop using getMode() and instead use functions
that explicitly check only the file type bits.

Reviewed By: bolinfest

Differential Revision: D5501256

fbshipit-source-id: c989dab2fdacb5b9cdecb6f5101795298f57e78b
2017-07-26 13:21:13 -07:00
Saurabh Singh
55f92fe95d Making public fields of the TreeInode::Entry private
Summary:
Before this commit, TreeInode::Entry was a struct which had two
private members: mode and inode. In this commit,

  1. TreeInode::Entry was changed from struct to class.
  2. Appropriate getters and setters were introduced for the public members to
     make them private.
  3. Existing code accessing the public members directly was modified to use
     the getters instead.
  4. A couple of TODOs were added to address Overlay::saveOverlayDir()'s access
     of child inode information.

Reviewed By: simpkins

Differential Revision: D5483953

fbshipit-source-id: 50d526731e193a3a4a32742bd4d49deb9ee6b432
2017-07-25 20:12:59 -07:00
Victor Gao
a477e9663f comment out unused parameters
Summary: This uses `clang-tidy` to comment out unused parameters (in functions, methods and lambdas) in fbcode. Cases that the tool failed to handle are fixed manually.

Reviewed By: igorsugak

Differential Revision: D5454343

fbshipit-source-id: 5dee339b4334e25e963891b519a5aa81fbf627b2
2017-07-21 15:01:05 -07:00
Eamonn Kent
6141d8ba20 Use FB_LOG in order to allow category logging (by mount path)
Summary:
Log messages to an eden.strace category.
This allows us to enable/disable based on the mount_path. For example:

./buck-out/gen/eden/cli/cli.par daemon -F -- --logging  eden/strace/data/users/ekent/eden-NEW=DBG7

Thus, we are using a category, rather than filename (default)

Reviewed By: bolinfest

Differential Revision: D5464387

fbshipit-source-id: 6a54badd6bb806cfcda1742fd970073d91303396
2017-07-21 06:54:24 -07:00
Jyothsna Konisa
371d7f889f Refactoring FileInode FileData.
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
2017-07-17 13:20:45 -07:00