Summary: Begin tracking pids passed into FUSE in the ProcessAccessLog.
Reviewed By: strager
Differential Revision: D9595795
fbshipit-source-id: 02e5fefebcd0de860274409ba6258f14fa050b55
Summary:
Part of the larger project to modify Future<T>::then to be r-value qualified and use Future<T>::thenTry or Future<T>::thenValue.
The goal is to disambiguate folly::Future and to improve type and lifetime safety of Future and its methods.
Codemod:
future<T>.then(callable with operator()(not-a-try)) to future<T>.thenValue(callable with operator()(not-a-try)).
future<T>.then(callable with operator()()) to future<T>.thenValue(callable with operator()(auto&&)).
future<T>.then(callable with operator()(auto)) to future<T>.thenValue(callable with operator()(auto)).
Reviewed By: Orvid
Differential Revision: D9696716
fbshipit-source-id: d71433c75af8422b2f16733c0b18a417d5a4cf2e
Summary:
It looks like D9477181 conflicted with D9635507, causing a compile error:
```
eden/fs/utils/ProcessNameCache.cpp:74:15: error: no member named 'names' in 'folly::LockedPtr<folly::Synchronized<facebook::eden::ProcessNameCache::State, folly::SharedMutexImpl<false> >, folly::LockPolicyExclusive>'
state.names.emplace(pid, ProcessName{detail::readPidName(pid), now});
~~~~~ ^
eden/fs/utils/Synchronized.h:48:10: note: in instantiation of function template specialization 'facebook::eden::ProcessNameCache::add(pid_t)::(anonymous class)::operator()<folly::LockedPtr<folly::Synchronized<facebook::eden::ProcessNameCache::State, folly::SharedMutexImpl<false> >, folly::LockPolicyExclusive> >' requested here
return update(wlock);
^
eden/fs/utils/ProcessNameCache.cpp:58:3: note: in instantiation of function template specialization 'facebook::eden::tryRlockCheckBeforeUpdate<folly::Unit, facebook::eden::ProcessNameCache::State, (lambda), (lambda)>' requested here
tryRlockCheckBeforeUpdate<folly::Unit>(
^
eden/fs/utils/ProcessNameCache.cpp:81:15: error: no member named 'waterLevel' in 'folly::LockedPtr<folly::Synchronized<facebook::eden::ProcessNameCache::State, folly::SharedMutexImpl<false> >, folly::LockPolicyExclusive>'
state.waterLevel += 2;
~~~~~ ^
eden/fs/utils/ProcessNameCache.cpp:82:19: error: no member named 'waterLevel' in 'folly::LockedPtr<folly::Synchronized<facebook::eden::ProcessNameCache::State, folly::SharedMutexImpl<false> >, folly::LockPolicyExclusive>'
if (state.waterLevel > state.names.size()) {
~~~~~ ^
3 errors generated.
```
Dereference `state` to fix the error.
Reviewed By: chadaustin
Differential Revision: D9673324
fbshipit-source-id: 5143cd22baf66307e5aacb0a04669de69dde99e8
Summary:
Add a ProcessAccessLog that supports cheaply tracking pids passed
into FUSE. It's a write-many, read-maybe access pattern, so the code
is careful to add as little latency as possible on the hot path.
Reviewed By: strager
Differential Revision: D9477839
fbshipit-source-id: 6928c1d09d55c2b0c0958ac2cb0dc91ec21b370c
Summary: While looking at FuseChannel I noticed an unnecessary Future.
Reviewed By: strager
Differential Revision: D9595672
fbshipit-source-id: 5c84822c4f2c4c3c78b88456e44728e463d5a1e8
Summary:
Eden is accessed by many short-lived processes. We want to show their
names in `eden top` so efficiently try to grab the names of pids. This
code will be used in a later diff.
Reviewed By: strager
Differential Revision: D9477181
fbshipit-source-id: 28998ac8bf7b804a3bd4944fbda4c7d1a0918312
Summary:
For `eden top`, we want to count accesses by pid by the second for the
past N seconds. Remembering forever would be too expensive so add a
circular data structure that forgets old entries on insertion and
reads.
Reviewed By: strager
Differential Revision: D9477166
fbshipit-source-id: 7019d441fde0cf40d8f5b08ed8dc13fafe0cbcac
Summary:
This method is called on every lookup and getSha1 (anything
that seeks a path) and can be a pretty contended lock, so let's make
it optimistically attempt a read lock.
Reviewed By: chadaustin
Differential Revision: D9635505
fbshipit-source-id: c84751efb7a952b2cf458e06ffc8ee670638f9bb
Summary:
Without this, you cannot use this method for non-copyable
types (e.g. Future)
Reviewed By: chadaustin
Differential Revision: D9635506
fbshipit-source-id: e57f1ae79acd544e5db0557d4654b675a419304c
Summary:
By passing a locked ptr instead of a reference directly to
the locked object, the check and update methods can drop the lock
early
Reviewed By: chadaustin
Differential Revision: D9635507
fbshipit-source-id: a881043cfd2c28f6f53eb12e1494fcbc5f7f8e08
Summary:
Every 10 minutes, unload any inodes that have not been accessed within
6 hours. This work helps keep Eden's memory usage from growing to O(repo) over
time.
Reviewed By: wez
Differential Revision: D6766900
fbshipit-source-id: af4af8fe12e5f43ce90f26578fa9bd534f124085
Summary:
Modified the build scripts to use dependencies from D:\edenwin64. This is the location where we will mount the edenwin64.iso.
It also contains changes to compile it with the latest Eden Linux changes plus removed some POC stuff and dependencies from my laptop.
Reviewed By: strager
Differential Revision: D9545688
fbshipit-source-id: e92e34d0af07974845faf9f729e0861fde5af459
Summary:
Change how fsck reports orphan inodes so that it only creates a single Error
object tracking all orphan inodes.
This will make it easier to write code to fix the orphan inodes as we will
want to process all of the orphan inodes together.
Reviewed By: wez
Differential Revision: D9615337
fbshipit-source-id: 452d0e67f357b8b2ac3b24d89e23fffb5bd816fd
Summary:
Wait to print errors until after we have finished scanning all of the overlay
files. Previously we printed errors as they were detected. However we would
not be able to print the path names correctly if we had not processed all of
the inode's parents yet. This resulted in error messages incorrectly
displaying paths as unlinked in some cases.
Also improve the error strings shown for MissingMaterializedInode and
InvalidMaterializedInode to also include the inode number. Also correct the
inode type display for InvalidMaterializedInode.
Reviewed By: wez
Differential Revision: D9615336
fbshipit-source-id: eb273d51c937e76ffed0e021da848f5fb940145d
Summary:
Add an --overlay flag to the `eden fsck` command to allow running it on an
alternative overlay directory.
This is similar to the `--overlay` flag added to the `eden debug overlay`
command in D9445560.
At the moment a checkout path is still required when running
with `--overlay`. This is used to find the socket to talk to edenfs. This is
not actually used at the moment, but in the future we may need this in order
to help fix some of the filesystem state in some cases. (For instance to get
the current blob or tree ID for a path in the current commit.)
Reviewed By: wez
Differential Revision: D9615334
fbshipit-source-id: 5a0da55f00429f596be5d86e5303e1fcdeff9ea7
Summary:
Unfortunately, our current config parsing implementation is
forcing us to stringly-type all data, which partially defeats one
of the benefits of using a modern typed configuration file format
such as TOML.
This means that boolean options have to be specified as strings in
the config file.
I don't want to ship the Mononoke feature integration out and tell
folks to use `"true"` to turn it on as it will impose a future migration
concern on us.
This diff does the simple but gross thing: at parse time, rather than
ignoring boolean values, we convert them to string so that the upper
layer can convert them to bools again.
Reviewed By: strager
Differential Revision: D9582582
fbshipit-source-id: a8eb8a8e4b59e3d74b3371b743a6eb590b3b0f58
Summary:
Since the privhelper process is often not our direct child
we both cannot and do not need to wait for it, so treat ECHILD as
a successful exit status to prevent bubbling up an exception that
blocks graceful restart.
Reviewed By: simpkins
Differential Revision: D9581473
fbshipit-source-id: 6d53bbb6ee2043de76df9c6870028a1c15571dac
Summary:
This test seems to fail frequently while waiting on the tree data for the
"src" tree (at line 182). The code previously was using a 1s timeout for this
operation; this changes it to use a 10s timeout.
I'm not sure what would have changed that makes this take longer now. It's
possible something changed with the treemanifest code on the mercurial side.
It is somewhat surprising to me that this would regularly take more than 1s
though since this is only computing data locally about a pretty small test
repository.
Reviewed By: wez
Differential Revision: D9583645
fbshipit-source-id: 24695cc1b940540ec32461e2d80c30a3fd87a3e3
Summary:
Refactored the existing fuse latency method for reuse and
added logic to pull out the thrift request latency stats.
Reviewed By: simpkins
Differential Revision: D9502587
fbshipit-source-id: b93d496c33a17efea7539f149c5db749285d75c4
Summary: This let's us track latency of thrift calls as a histogram rather than just avg. Any reason I should just add this for all the methods? I'll also follow up with some cli change to report them.
Reviewed By: simpkins
Differential Revision: D9485606
fbshipit-source-id: 2c666bf45846bdc1fe8cc33d557ac22d07f310e2
Summary:
Open the lock file with O_CLOEXEC so that the `hg_import_helper.py`
subprocesses do not inherit the lock file.
Reviewed By: wez
Differential Revision: D9553355
fbshipit-source-id: b7024e9637bc52ef9a8a01cff60df00d8617a839
Summary:
This check doesn't really matter: no matter what the read returns this process
should exit. However this change is required in order for us to update
`folly::readFull()` to warn if the return value is not used.
Reviewed By: wez
Differential Revision: D9544151
fbshipit-source-id: 6813bb52460ca3a24969ce3f677dd3e2c0275a16
Summary:
Add a new unit test that exercises HgImporter and HgBackingStore using an
alternate hg_import_helper.py script that can inject errors into the response
data stream. This tests the behavior of HgImporter when there are
serialization errors with the data, as opposed to legitimately-serialized
error responses containing a python exception message.
Reviewed By: wez
Differential Revision: D9510568
fbshipit-source-id: ae86f69d5559701e9e8a9a18569986d64488eaf4
Summary:
Overall plan to modify Future<T>::then to be r-value qualified and use Future<T>::thenTry or Future<T>::thenValue.
The goal is to disambiguate folly::Future and to improve type and lifetime safety of Future and its methods.
Codemod:
* future<T>.then(callable with operator()(not-a-try)) to future<T>.thenValue(callable with operator()(not-a-try)).
* future<T>.then(callable with operator()()) to future<T>.thenValue(callable with operator()(auto&&)).
* future<T>.then(callable with operator()(auto)) to future<T>.thenValue(callable with operator()(auto)).
Reviewed By: yfeldblum
Differential Revision: D9523023
fbshipit-source-id: b59a5fd4b155a3d9c2c85d157c436399627a856b
Summary:
Overall plan to modify Future<T>::then to be r-value qualified and use Future<T>::thenTry or Future<T>::thenValue.
The goal is to disambiguate folly::Future and to improve type and lifetime safety of Future and its methods.
Codemod:
* future<T>.then(callable with operator()(not-a-try)) to future<T>.thenValue(callable with operator()(not-a-try)).
* future<T>.then(callable with operator()()) to future<T>.thenValue(callable with operator()(auto&&)).
* future<T>.then(callable with operator()(auto)) to future<T>.thenValue(callable with operator()(auto)).
Reviewed By: yfeldblum
Differential Revision: D9512177
fbshipit-source-id: daa3581611dcd9f32d9314bae1c5fa0f966613f3
Summary:
Update hg_import_helper.py to throw a new ResetRepoError type when it decides
that its local mercurial state is invalid and that it needs to reopen the
repository. The C++ code will catch this exception, restart the
hg_import_helper.py and retry the request. (It will only retry once for each
request.)
The previous behavior of simply closing and re-opening the repository object
in Python has been problematic, as this has resulted in resource leaks. The
hg_import_helper.py process itself ends up leaking memory when re-opening the
repository. This also appears to result in many scmmemcache helper processes
being created and not cleaned up. Presumably these are associated with the
old repository state and not cleaned up properly when we reopen the
repository.
Reviewed By: quark-zju
Differential Revision: D9510664
fbshipit-source-id: 449dfa9e2e21aabf8b3ce640749d32aa8f8e4052
Summary:
Update the HgImporter code to actually check the transaction ID field in the
response headers from the hg_import_helper script.
Reviewed By: wez
Differential Revision: D9507288
fbshipit-source-id: 0a170bd6b39426176c938a135508fa4bb928053a
Summary:
Goal: change catchErrors() so its param is an rvalue-ref (`template <typename T> ... catchErrors(Future<T>&& fut)`) instead of a forwarding ref (and, if necessary, adjust callsites that passed an lvalue-ref).
Rationale: this is part of T33085035 which is migrating Future::onError() to be rvalue-only. D9505757 codemodded `RequestData::catchErrors()` from `fut.onError()` to `std::move(fut).onError()`. However `fut` was a forwarding-ref so it can actually hold an lvalue-ref.
Timeline/urgency: this diff is a prereq to removing the lvalue-qual overload of Future::onError() (T33085035). (Technically the only part of this that is a prereq to T33085035 is to adjust callers of catchErrors() that pass lvalue Futures, but changing the catchErrors() signature is righteous since it will force a more sensible error-message on any future callers which erroneously pass an lvalue.)
Note: D9505757 landed since it didn't break anything, even though it used `std::move()` on a forwarding-ref. (The lvalue-qual and rvalue-qual overloads of Future::onError() currently have identical semantics; neither invalidates / moves-out its Future object.)
Reviewed By: yfeldblum
Differential Revision: D9510204
fbshipit-source-id: 5c1ff2f36c5d84c583268f5e952fd322ea8e9327
Summary:
Update the HgImporter code to check the return value from
`folly::writevFull()`
Reviewed By: wez
Differential Revision: D9507289
fbshipit-source-id: 00985f50ebb4717f7b97a199bf9efba6b4f5f628
Summary:
Previously the HgImporter code was not checking the return value of
`folly::readFull()`. This function does not throw exceptions: it may return
-1 on error or a partial number of bytes read. Previously the HgImporter code
did not perform any checking of the result from this function.
Reviewed By: wez
Differential Revision: D9507287
fbshipit-source-id: 80d8bdb54375eb287d1473675fffb48c4cd34c56
Summary:
These methods were only used in the test utility in one location. I plan to
clean up how the HgImporter code reads from the underlying python process, and
it will be easier if these are non-static methods.
This removes the ability from the test utility to read from a pre-computed
manifest data file. I don't think we care too much about this functionality
any more--most of our repositories are exclusively using treemanifest mode now
anyway.
Reviewed By: wez
Differential Revision: D9507290
fbshipit-source-id: 97fe3fc9d8574c97a41328ed42e7afdacb3734d1
Summary:
Add a wrapper class around HgImporter that can catch errors communicating with
the underlying `hg_import_helper.py` script. If something goes wrong
communicating with the python code we restart the import helper and retry
the operation once.
This code only retries on HgImporterError exceptions, and nothing throws these
at the moment. Therefore for now this should not result in any behavior
changes. In the future I will update our error handling to throw
HgImporterError when we fail to communicate with the underlying
hg_import_helper.py script.
Reviewed By: wez
Differential Revision: D9507291
fbshipit-source-id: 0167959df6292a577942e049576669fb6711ce76
Summary:
This is part of "the great r-valuification of folly::Future":
* This is something we should do for safety in general.
* Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
* This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
* This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
* The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).onError(...)` instead of `f.onError(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
Codemod changes:
* expr.onError(...) ==> std::move(expr).onError(...) // if expr is not already an xvalue
* expr->onError(...) ==> std::move(*expr).onError(...)
Note: operator precedence of that last step is safe - no need to parenthesize `expr`. Reason: `->` binds more tightly than unary `*`.
Reviewed By: yfeldblum
Differential Revision: D9505757
fbshipit-source-id: de666f3a877313526d10f5d3569a1bbb2203f066
Summary:
D9494651 and D9415207 logically conflicted with each other, but did not have
conflicts at the source code level. They both landed independently which
resulted in a broken build even though CI had passed on their individual
diffs. This updates them to be compatible.
Reviewed By: wez, strager
Differential Revision: D9505360
fbshipit-source-id: 72743167d934d1d57044cda8d5a7af03346ab142
Summary:
Change HgImporter::importFileContents() to return a Blob object rather than
just the IOBuf.
This shouldn't have any behavior changes. However, it might help track down
some of the cases where we have seen files get incorrectly imported with empty
contents in some rare cases. We're not sure what is causing that, but
incorrectly using a moved-from IOBuf object would be one thing that might
produce this behavior. By changing this code to a `unique_ptr<Blob>` will
will eliminate the possibility of seeing empty buffer contents if some code
path is somehow ever able to use one of these objects after they have been
moved away from. (The code would end up with a null pointer in this case
rather than an empty buffer. If this ever does occur in practice a null
pointer would make the problem more obvious than an empty buffer.)
Reviewed By: chadaustin
Differential Revision: D9494651
fbshipit-source-id: 799cc1b1d0e2ac32ae2013dc80c84ec17cc1c491
Summary:
This updates the `hg_import_helper.py` code so that when importing files it
also includes the file length in the response, in addition to the response
contents itself. This information is redundant and exists purely to help
the receiving C++ code verify that the response is in fact completely valid.
This is implemented with a new command number, and `hg_import_helper.py` still
supports the old command number with the old behavior so that it will still
work if old edenfs daemon instances start new instances of
`hg_import_helper.py`.
Reviewed By: chadaustin
Differential Revision: D9494653
fbshipit-source-id: 34d1221b6048d6016b30fd7f606ca0b140552d80
Summary:
In D9486915 I updated the code to invoke `hg` from the local repository, but
this was invoking the telemetry wrapper, and this was still falling through
and running the system-installed `hg.real` binary rather than the from the
local repository. This fixes the code to also pass through the `HG_REAL_BIN`
and `CHG_BIN` environment variables when invoking the `hg` wrapper so that the
underling real hg code is picked up from the current repository as wel.
Reviewed By: chadaustin
Differential Revision: D9494271
fbshipit-source-id: 89c5154f501536b9d7a43d472b1204f10e5dc664
Summary: This commit makes eden ask Mononoke API Server first when it is fetching blobs.
Reviewed By: chadaustin
Differential Revision: D9415207
fbshipit-source-id: 1146a9b6295e8ddb15e4bcb098379ea75886baa5
Summary:
The desired end state for how next inode number gets passed across
takeover. This should not land as is - the prior diffs need to land
and wait for a while until everyone transitions.
Reviewed By: simpkins
Differential Revision: D8195550
fbshipit-source-id: 2fc40f881cc1a331df95ef99f7e9e4f2e69c8643
Summary:
Prior to this diff, to read the inode metadata from an InodeBase, you
had to cast it to a FileInode or TreeInode. Add a public getMetadata
call that acquires the appropriate lock depending on whether it's a
FileInode or TreeInode.
Reviewed By: simpkins
Differential Revision: D9327180
fbshipit-source-id: 66594fb639f1ea72a0e48dcb79234e14d87ec4a2
Summary:
Update the TARGETS file for eden/fs/store/hg/test to make sure that they
explicitly specify EDEN_HG_BINARY in the environment so that the test code
will run against hg from the local repository rather than the system-installed
mercurial.
Also update testharness/HgRepo.cpp to use this environment variable when
searching for the hg binary.
Reviewed By: chadaustin, wez
Differential Revision: D9486915
fbshipit-source-id: f8f8a1acde9e8b0a032a026fa477b21c70afacbc
Summary: The error message for FLAGS_etcEdenDir should contain its value.
Reviewed By: chadaustin
Differential Revision: D9484715
fbshipit-source-id: edecf42c74830a672ab9b3cdfac357b9abdedaf8
Summary:
Fix the code in `eden_dirstate._call_match_callbacks()` to correctly match
ignored symlinks that are explicitly listed in the matcher's file list.
It looks like this was just a mistake in the original code on my part: I
intended to check both `S_ISREG()` and `S_ISLNK()` but instead incorrectly put
the check for `S_ISREG()` twice.
Reviewed By: wez
Differential Revision: D9476524
fbshipit-source-id: 67e0fa7c2fbaac97598a8e2d028c9ef0999ed88a
Summary: Modifies a few callsites in Eden that indirect through a separately allocated lambda to replace Future::then with Future::thenValue.
Reviewed By: yfeldblum
Differential Revision: D9485137
fbshipit-source-id: 84fa03a736a0faa162372e221625d82813a68620
Summary:
When we load an empty blob from the LocalStore double check with the
BackingStore to confirm that it should actually be empty.
We have seen multiple instances of files being incorrectly imported as empty.
So far this error has always been fixed by a re-import. We still haven't
tracked down the root cause, but this change should help workaround the issue
by ensuring that we double check the file contents before returning the data.
Reviewed By: chadaustin
Differential Revision: D9476522
fbshipit-source-id: 6d57cf15c42736ecbcb106a731259b77db06d8f1
Summary:
`F14NodeMap` has proven to be a safe and more performant drop-in replacement
for `std::unordered_map`. With the ability to do heterogeneous lookup and
mutation, we no longer need `StringKeyedUnorderedMap`, whose main purpose was
to avoid unnecessary string copies during lookups and inserts.
Reviewed By: nbronson
Differential Revision: D9124737
fbshipit-source-id: 81d5be1df9096ac258a3dc1523a6a0f5d6b9e862
Summary:
D9389865 changed the rel_path variable from a string to a pathlib.Path object.
However it missed a couple places where the path object needed to be converted
to bytes.
Reviewed By: chadaustin
Differential Revision: D9476525
fbshipit-source-id: 2dc69f5a0378ffbc02b2724a097604df288683c9
