Summary:
D13853115 adds `edenscm/` to `sys.path` and code still uses `import mercurial`.
That has nasty problems if both `import mercurial` and
`import edenscm.mercurial` are used, because Python would think `mercurial.foo`
and `edenscm.mercurial.foo` are different modules so code like
`try: ... except mercurial.error.Foo: ...`, or `isinstance(x, mercurial.foo.Bar)`
would fail to handle the `edenscm.mercurial` version. There are also some
module-level states (ex. `extensions._extensions`) that would cause trouble if
they have multiple versions in a single process.
Change imports to use the `edenscm` so ideally the `mercurial` is no longer
imported at all. Add checks in extensions.py to catch unexpected extensions
importing modules from the old (wrong) locations when running tests.
Reviewed By: phillco
Differential Revision: D13868981
fbshipit-source-id: f4e2513766957fd81d85407994f7521a08e4de48
Summary:
Extend EdenConfigParser to read string arrays from config files. TOML already supports arrays; this diff just makes the arrays accessible to CLI code as Python sequences.
Currently, no code needs to read string arrays. This feature will be used in the future for the service.start_systemd_manager_command config option:
[service]
start_systemd_manager_command = ['sudo', 'systemctl', 'start', 'user@${USER}.service']
Reviewed By: simpkins
Differential Revision: D13708911
fbshipit-source-id: 74e9e03ded9896a84a28611dc1df589d8fcb7597
Summary:
Added functions to Eden Doctor to check for insufficient disk space available on the Eden mount points used by all checkouts.
Relevant Eden directories are retrieved automatically and the mount point is checked for each folder. Thus, it only reports excessive disk usage at most once per mount point.
All this functionality has also been factored out into the check_filesystems.py file (former check_using_nfs.p) to keep similar functionality in the same module.
The 90% threshold is hardcoded for now but could also be made customizable via Eden's config (e.g. EdenInstance.get_config_value and /etc/eden/config.d/doctor.toml) and triggers an advice problem, unless the total free disk space is below 1GB on any mount point of non-zero size.
Also added state_dir to FakeEdenInstance to fix bug in test cases which accesses state_dir and attached test cases for the problem generation logic due to disk usage.
Reviewed By: strager
Differential Revision: D13732678
fbshipit-source-id: a1bbb8ffa0bfdc816611dfd628bed62485e54294
Summary:
The service.experimental_systemd config option is implemented hackily: the value is parsed as a boolean, then converted to a string (with `str`), then compared with `"True"`. Remove this ugliness by not converting all config options to strings in EdenConfigParser, and adding a type-safe `get_bool` accessor.
Aside from disallowing `experimental_systemd = "True"` in configs and changing the output of `eden config`, this diff should not change behavior.
Reviewed By: simpkins
Differential Revision: D13666331
fbshipit-source-id: 65b7d233c43db08bc7e0288f248041ca4a18a5fd
Summary:
Move top-level Python packages `mercurial`, `hgext` and `hgdemandimport` to
a new top-level package `edenscm`. This allows the Python packages provided by
the upstream Mercurial to be installed side-by-side.
To maintain compatibility, `edenscm/` gets added to `sys.path` in
`mercurial/__init__.py`.
Reviewed By: phillco, ikostia
Differential Revision: D13853115
fbshipit-source-id: b296b0673dc54c61ef6a591ebc687057ff53b22e
Summary:
This adds a new `CheckoutInfo` class in the `eden doctor` code to consolidates
some information about a particular checkout. This should make it easier to
pass around this single object to various check functions, instead of several
separate arguments.
I did make a few functional changes as part of this refactoring:
- All checkouts are now checked in order sorted by their mount path.
Previously non-mounted checkouts were checked before running checkouts.
- The code now reports additional errors about checkouts if they are running
but not listed in the on-disk configuration, or if their running state
differs from the on-disk configuration in some cases.
Reviewed By: strager
Differential Revision: D13526754
fbshipit-source-id: 81802f96a3a3c9f2aaad9a4468fb666ed8b2e47e
Summary:
Update "eden doctor" to just check if its main state directory is on NFS
rather than checking if the state directory for each checkout is on NFS.
It's quite unlikely that Eden's state directory spans multiple devices on both
NFS and non-NFS, given that Eden itself generally creates this directory
structure. Therefore it seems unnecessary to individually check different
checkout subdirectories inside this structure. We also should report a
problem if the state directory is on NFS even if there currently aren't any
checkouts configured.
This also provides remediation text telling users the most likely cause for
having their state directory on NFS, and how they can probably fix it.
Reviewed By: wez
Differential Revision: D13831086
fbshipit-source-id: f9f243898fbfe3dcb581678a6ec8a63a29855c5e
Summary:
In preparation for proper support for booleans and arrays in configs, refactor the interface of `EdenConfigParser`:
* Rename `get` to `get_str` so callers explicitly mention the expected type. This change will make sense in the future when `EdenConfigParser` supports boolean and array options in addition to string options. Also, rename the `fallback` parameter to `default`.
* Move `config_to_raw_dict` into `EdenConfigParser` and remove the `raw` boolean parameter from `items`.
* Rename `items` to `get_section_str_to_str`, and change its return type to `Mapping` (instead of `List[Tuple]`).
This diff should not change behavior.
Reviewed By: simpkins
Differential Revision: D13661228
fbshipit-source-id: fc93560f95b75ade78044d8146e92c6f65dfc404
Summary:
Python's `configparser` module requires values to be strings. Values in TOML can have other types, such as boolean and array, and `configparser` gets in the way of parsing such values.
Allow fixing `configparser`'s limitations by rewriting parts of it. Instead of using `configparser.ConfigParser`, create an `EdenConfigParser` class (which implements a subset of `configparser.ConfigParser`'s API) and use that instead.
This diff should not change behavior. A future diff will enable non-string config values.
Reviewed By: simpkins
Differential Revision: D13646873
fbshipit-source-id: e26a886789cf00dc363f3a97fe0febe6f5010f8e
Summary: using upgrade script to clear out all remaining version-set configs
Reviewed By: dark
Differential Revision: D13832474
fbshipit-source-id: 52c280cbd79b1410821ed829465b1c0907b50a86
Summary: PrivHelperServer::initLogging is declared in the .h file, but it's never defined. Remove the useless declaration.
Reviewed By: simpkins
Differential Revision: D13814416
fbshipit-source-id: 25cb47442a19947da08d13d9bed9b4631a1c9739
Summary:
`EdenInstance.get_config_value` raises a KeyError if the option or section is not set in any config files. Every caller of `EdenInstance.get_config_value` handles KeyError manually. Simplify code by letting callers specify a default value.
Aside from treating some options with an empty value as if they were unset, this diff should not change behavior.
Reviewed By: chadaustin
Differential Revision: D13737806
fbshipit-source-id: fdb7fa75d601de4644704813db38b671b27f73d7
Summary:
Update the `eden list` command to also report the current state for each
checkout if it is not running normally. Also added a `--json` flag to
print information as JSON so it can be consumed programmatically.
Reviewed By: strager
Differential Revision: D13503053
fbshipit-source-id: 4ef366f5bf4a1157036fdfd7ff1056079588e802
Summary:
The code that parses the kernel version requires a fairly strict
structure and not all kernels match that structure. If the number
doesn't parse, skip the checks.
Reviewed By: wez
Differential Revision: D13824190
fbshipit-source-id: 8eb2ea2778e1e470d7f7708fc256dd5fae5a02b4
Summary:
This patch enables BST stats by default, and works as follows:
- it uses quantile stats to maintain per queue per port stats.
- these stats are exposed to ODS.
- experiments showed that a significant CPU cycles were spent in doing
bcm_cosq_bst_stat_sync which syncs the hw stat value to sw copy.
For example, calling bcm_cosq_bst_stat_sync while retrieving BST stats
for for every port/every queue/every second resulted into CPU utilization
going up from ~ 50% for Wedge 100 to ~100-120%. Thus, this patch calls
bcm_cosq_bst_stat_sync only once per minute and then retrieves all BST stats.
- The above optimization also helps when bufferstatlogging is enabled.
- when bufferstatlogging is enabled, the BST stats will be retrieved every
second for scuba logging, and quantile stats will also be exported every
second.
- The bufferstatlogging is enabled using fboss CLI (over thrift).
We no longer need enable_fine_grained_buffer_stats option. But, a new option
viz. update_bststats_interval_s is introduced that keeps the frequency of BST
stats updates configurable.
The virtual memory overhead of always maintaining BST stats is < 5%, and
physical memory overhead is < 0.1%. Thanks to the sync optimization above,
the CPU overhead is negligible. Refer Test Plan for raw data and math.
An alternative is to explore using TimeSeriesMinMax (to maintain max over a
time window) and TLCounter (to expose stats to ODS) instead of quantile stats.
One benefit of using quantile stats is that we don't need to maintain our own
TimeSeriesMinMax but can rely on common facebook wide infra. Moreoever,
quantile stats interface is (arguably) simpler than using combination of
TimeSeriesMinMax + TlCounter'.
A separate diff against configerator is sent out to expose the new bst_stats: D8902636
Differential Revision: D8764329
fbshipit-source-id: 78db3de5340497f2f5780863035dd9ff28dcd79b
Summary: Currently, `runOnDestruction` aims to be thread-safe; new callbacks are added to the `onDestructionCallbacks_` list while the associated mutex is held. However, the caller may own the `LoopCallback` and wish to destroy/cancel it before the `EventBase` destructor runs, and this callback cancellation is not thread-safe, since unlinking does not happen under the lock protecting `onDestructionCallbacks_`. The primary motivation of this diff is to make on-destruction callback cancellation thread-safe; in particular, it is safe to cancel an on-destruction callback concurrently with `~EventBase()`.
Reviewed By: spalamarchuk
Differential Revision: D13440552
fbshipit-source-id: 65cee1e361d37647920baaad4490dd26b791315d
Summary:
Now that the deadlock in ProcessNameCache has been fixed, bring it
back.
Reviewed By: strager
Differential Revision: D13742965
fbshipit-source-id: f407105e06b9954766bdb48ef1303e2003c07284
Summary:
This fixes a deadlock where the kernel held the memory manager lock while satisfying a page fault resulting in a call into Eden, which tried to read /proc/pid/cmdline, acquiring the memory manager lock again.
The fix is to never read /proc/pid/cmdline while handling a request. That work is now done on a background thread.
Reviewed By: strager
Differential Revision: D13685318
fbshipit-source-id: c4e17de3358b668638db0c2dcfba8536d05e5b7c
Summary:
TemporarySystemdUserServiceManagerTest.test_exit_kills_manager is flaky due to a race condition. SystemdUserServiceManager.exit does not wait for the systemd process to exit; I think it only waits for systemd to close its socket. This means the process can still be alive, and `did_process_exit` can return true.
Fix the race condition by making SystemdUserServiceManager.exit block until the systemd process exits.
Reviewed By: chadaustin
Differential Revision: D13791407
fbshipit-source-id: 8422e0101eaea8b4da285dcb0fcf564435b30065
Summary:
Most of these calls were unnecessary, as the data wasn't indented.
Only one place was actually using the `dedent()` functionality, but I
changed that to not use it either, just to be consistent with the other parts
of the code.
Reviewed By: strager
Differential Revision: D13744295
fbshipit-source-id: c2a84b6fc979138559fbd6fc04841409d69c204b
Summary:
Previously edenfs always returned `ALIVE` in response to the fb303
`getStatus()` call. This fixes the code to return `STARTING` while Eden is
still starting and `STOPPING` while Eden is shutting down.
Reviewed By: strager
Differential Revision: D13515856
fbshipit-source-id: 91fe52e5f7f0a9e2c48ca1dd7663c99319afa8ad
Summary: This is needed to avoid link errors for code that uses `EDEN_BUG()`
Reviewed By: wez
Differential Revision: D13806142
fbshipit-source-id: a5baea17830629e1271351ddd127118789681470
Summary: catching this up to distillery version
Reviewed By: dark
Differential Revision: D13798690
fbshipit-source-id: 6d222d9378b52b173208103b643ceb1e3bf73871
Summary:
I want to add an array config option to Eden. It's unclear how arrays should be printed by 'eden config --get'.
'eden config --get' is not used anywhere. Instead of figuring out the right way to print arrays, just kill support for the --get option.
If someone *is* using 'eden config --get', make it obvious and greppable that it's now deliberately broken.
Reviewed By: chadaustin
Differential Revision: D13737807
fbshipit-source-id: 7d7e2d0d5e90641c53b2caa463fc9e1854416745
Summary:
I plan on replacing Eden's use of `configparser` [1]. Prepare for that change by reducing the set of methods Eden uses from `configparser`. In particular, avoid using `configparser.SectionProxy` and `configparser.ConfigParser.__getitem__`.
This diff should not change behavior.
[1] Python's `configparser` module requires values to be strings. Values in TOML can have other types, such as boolean and array, and `configparser` gets in the way of using such values.
Reviewed By: simpkins
Differential Revision: D13646807
fbshipit-source-id: 12f9875b86e5fb639e7e6da5ef05bb9a1e4fc543
Summary:
As of D13737806, for most config options, an option with an empty value is treated as if it is unset. The `type` and `path` options in repository sections behave differently: an empty value is different from no value. Make `type` and `path` consistent with other config options.
This diff should not change behavior given valid config files.
Reviewed By: chadaustin
Differential Revision: D13737986
fbshipit-source-id: f1d2044229d828cc3a04ce46acc31e90bee76c39
Summary:
Add an option to forcibly kill `edenfs` with SIGKILL without ever attempting
to query it over thrift.
This should provide a way for users to reliably kill edenfs even if it is
hung. This shouldn't be necessary in most cases, but it lets us tell users to
run this command as a last resort if something is wrong.
Reviewed By: chadaustin, strager
Differential Revision: D13744188
fbshipit-source-id: 13378d04b3398e72ed3733d4ebb68b39868007bd
Summary:
When graceful restart was first implemented we forgot to update the
lock file with the new pid, resulting in occasional unexpected output
from tools like eden doctor.
Reviewed By: simpkins
Differential Revision: D13744411
fbshipit-source-id: cdc758ed6ac1201fd2ff3e9d7805bb5ab6f83e8a
Summary:
On a systemd-managed system, the `XDG_RUNTIME_DIR` environment variable is set on login. `systemctl` uses this variable to know how to talk to the systemd user manager. If `XDG_RUNTIME_DIR` is not set in the environment, `systemctl` (and thus `eden start`) fails with an unhelpful message:
Failed to connect to bus: No such file or directory
Improve this message by explicitly checking for the absence of `XDG_RUNTIME_DIR`.
Reviewed By: simpkins
Differential Revision: D13728111
fbshipit-source-id: a7f60fc29561acd05fbc1bf52d7968ae0e64d0c2
Summary:
The failure messages printed by 'eden start' are kinda crappy with systemd integration enabled. Add some tests for these messages so we can easily iterate on them.
In particular, test the following cases:
* The systemd user manager is no longer running
* The XDG_RUNTIME_DIR environment variable, needed to talk to systemd, is not set
* edenfs fails to start
This diff should not change behavior.
Reviewed By: simpkins
Differential Revision: D13723440
fbshipit-source-id: abae5c0e4a9f0bc6b8d0d606e8f5f36760aad5fa
Summary:
A bug in Pyre causes the properties of FindEXE to have an incorrect type. We currently work around this bug by silencing type errors. Unfortunately, this might silence legitimate errors too.
Instead of silencing type errors, using `typing.cast` to tell Pyre the correct type. This should expose legitimate errors if they exist.
This diff should not change behavior.
Reviewed By: chadaustin
Differential Revision: D13709138
fbshipit-source-id: 55f47f47062a35911c6bbe03ffd7b02a90a5107f
Summary:
The 'eden config' command prints output naively. This leads to confusing output when using the hg.extra_hgrc option. For example, in the following output, `mode = off` is part of hg.extra_hgrc's value, but it looks like it's a separate option in Eden's config:
```
$ eden config
[clone]
default-revision=master
[rage]
reporter=pastry --title "eden rage from $(hostname)"
[hg]
extra_hgrc=[fsmonitor]
mode = off
%include /etc/mercurial/repo-specific/eden.rc
[service]
experimental_systemd=True
```
Fix this issue by making 'eden config' output valid TOML:
```
[clone]
default-revision = "master"
[rage]
reporter = "pastry --title \"eden rage from $(hostname)\""
[hg]
extra_hgrc = "[fsmonitor]\nmode = off\n\n%include /etc/mercurial/repo-specific/eden.rc\n"
[service]
experimental_systemd = "True"
```
Reviewed By: simpkins
Differential Revision: D13661229
fbshipit-source-id: 76e4fa83ad186d04451623e3d8d87a78e4b821d8
Summary:
This is something that is not needed on linux because
the kernel module is typically already loaded (at least on the systems
on which we run).
On macos, since fuse is not part of the kernel, libfuse has some code
that loads it when needed.
This diff performs the equivalent actions for eden.
Reviewed By: simpkins
Differential Revision: D13721489
fbshipit-source-id: 627bc90681141d0e7da3d5b5e06756a36839958c
Summary:
This requires our mercurial repo to be available during
the build; I symlink it in alongside `common` in the `oss` dir,
and point it up to `scm/hg`.
This has partial support for mononoke too, but will need to add
logic to getdeps to pull down the proxygen repo and build that.
Reviewed By: simpkins
Differential Revision: D13480146
fbshipit-source-id: 54874245015af83a259f56944d2e5f87615baee7
Summary:
Note that the concept of bind mounts doesn't exist on macos, so that
portion of the server just throws.
Reviewed By: simpkins
Differential Revision: D13480147
fbshipit-source-id: 92225188c0af42574d090004490f3926d393747b
Summary:
In our linux deployments it was relatively straightforward
to import the mercurial runtime from a python process running the
system python executable. Our macOS deployments are a lot more
complex because they do not use the system python and do not install
the mercurial python packages in the python path of the target
python executable.
It is simpler to move the import helper functional into a mercurial
command that we can invoke instead of our own helper program.
This diff moves the script to be a debug command and adjusts its
argument parsing to match the mercurial dispatcher requirements.
There are some stylistic mismatches between this code and the
rest of mercurial; I'm suggesting that we ignore those as the
medium term solution is that this command is replaced by eden
directly consuming the rust config parsing code and by native
rust code to perform the data fetching that we need.
Reviewed By: pkaush
Differential Revision: D13522225
fbshipit-source-id: 28d751c5de4228491924df4df88ab382cfbf146a
Summary:
This updates the logic in EdenServer to add the EdenMount to the mountPoints_
map as soon as it is created, so that we track mount points as they are
initializing.
I don't expect this change to have any major impact in functionality yet. In
a subsequent diff I also plan have EdenServer keep mount points in the
mountPoints_ map longer while they are shutting down. I expect that change to
matter a bit more, as that will allow us to do a better job reporting and
debugging when mount points are taking a non-trivial amount of time to become
unreferenced and fully shut down.
Reviewed By: strager
Differential Revision: D13503050
fbshipit-source-id: 2e0e8dfde64c6a005efd6dcf503ad7577f314356
Summary:
There are some features of folly futures that are
currently being deprecated. Until that codemod lands, deprecation
warnings have been disabled in the buck build. To avoid
swamping the build output in the oss build, let's also turn
them off for cmake.
Reviewed By: strager
Differential Revision: D13686585
fbshipit-source-id: 14609a882bc78b7b31beb7ae02d762b9318e1312
Summary:
This is really a continuation of D13479516; the issue is that
the osxfuse kernel module is very eager to recycle `unique` request
id values, recycling them before our code has had a chance to update
internal state.
This diff re-keys the requests map so that we generate our own sequence
of identifiers to use as the key rather than the fuse protocol `unique`
value.
Because we cannot reliably track by `unique` value we also cannot
reliably implement interrupt support. We've never really tested
interrupt support, and it relies on functionality in folly futures
that hasn't really been tested or proven either, so I've removed
that functionality as part of this diff.
That allows simplifying some code in RequestData and FuseChannel;
we're now able to simply tack an `.ensure` on the end of the
future chain to ensure that we remove the entry from the map
once the future is resolved, successfully or otherwise.
Reviewed By: chadaustin
Differential Revision: D13679964
fbshipit-source-id: c1081a868c4061de2a725589ec1614959a8e9316
Summary:
Migrate the code from accessing optional Thrift fields directly to a safer
`optional_field_ref` API. See https://fburl.com/safe for more details.
The output of this codemod has been reviewed in D13259011.
To preserve semantics, each unchecked access is replaced with an explicit call
to `value_unchecked()`. If you are sure that accessing a field is safe (the
field is marked as set), you can later replace `value_unchecked()` with
`value()` or dereferencing (`operator *`):
```
ThriftStruct s = ...
- auto foo = s.foo_ref().value_unchecked();
+ auto foo = *s.foo_ref(); // will throw if s.foo is unset
```
Reviewed By: chadaustin
Differential Revision: D13684410
fbshipit-source-id: 919de4ddf89e7f0463f2614baba4bfbac1c8255c
Summary: the build breaks when making clean unless we declare this dep
Reviewed By: simpkins
Differential Revision: D13679633
fbshipit-source-id: f23a533eab9e37fdeab839e4f5e1b6b312ea10b0
Summary: This just makes the debug a little easier to follow.
Reviewed By: chadaustin
Differential Revision: D13680020
fbshipit-source-id: e4045822e56ba42a831ccb0ceaa9baaba5b79a10
Summary:
the osxfuse implementation includes some additional
fields in the `fuse_attr` struct. One of those fields is
`flags`. We were not initializing this field when converting
the stat data to fuse attributes which resulted in it holding
"random" data. In debug builds this seemed to usually end up
zeroed out but in release builds it was usually a bit pattern
that caused the kernel to respond with EPERM when attempting
to access the files. I didn't capture exactly what that
bit pattern was, just that initializing the struct to zeroes
reliably fixed up the EPERM issues in the Release build.
Reviewed By: chadaustin
Differential Revision: D13680004
fbshipit-source-id: 6b2ce6c10ef8f7db4a8a50bd3f2ddcfdddc3bb45
Summary:
The `make-client.py` script assembles an executable zip file
that holds all of the deps needed to run the `eden` cli on a posix
system.
Reviewed By: simpkins
Differential Revision: D13480144
fbshipit-source-id: de8cb093427c793a40e8bf81727f879216c9b41a
Summary:
This is necessary because the folly installed cmake config
advertises that it requires glog::glog but doesn't specify how to
find it.
See also D13482288
Reviewed By: simpkins
Differential Revision: D13486767
fbshipit-source-id: 0713ae70a1863fc23a5e86c21e8f72e3ba9e4ed2
Summary: Small things I noticed while working on other stuff.
Reviewed By: strager
Differential Revision: D10055671
fbshipit-source-id: de8c3b04928567a821172e6fa7ee0e056958e1e7
Summary:
If edenfs is not running or is unhealthy, 'eden rage' does not run 'eden doctor'. This means 'eden rage' does not include helpful output such as ~/local/.eden/clients/* being on NFS, or the Linux kernel version being unsupported.
Make 'eden rage' run 'eden doctor' regardless of the health of edenfs.
Reviewed By: simpkins
Differential Revision: D13633381
fbshipit-source-id: 2439057ba7a7bbe5041991ddc4ede256e86634f3
Summary:
Address this error with clang:
```
In file included from /Users/wez/fbsource/fbcode/eden/oss/eden/fs/service/main.cpp:25:
/Users/wez/fbsource/fbcode/eden/oss/eden/fs/fuse/privhelper/PrivHelper.h:22:1: warning: class 'Unit' was previously declared as a struct [-Wmismatched-tags]
class Unit;
^
/Users/wez/fbsource/fbcode/eden/oss/external/install/include/folly/Unit.h:36:8: note: previous use is here
struct Unit {
^
/Users/wez/fbsource/fbcode/eden/oss/eden/fs/fuse/privhelper/PrivHelper.h:22:1: note: did you mean struct here?
class Unit;
^~~~~
struct
```
Reviewed By: strager
Differential Revision: D13602383
fbshipit-source-id: 6e69716498680660181ab441c3c007b074ec1d40
Summary: The create_no_such_repository_exception method is not referenced anywhere. Delete it.
Reviewed By: chadaustin
Differential Revision: D13646638
fbshipit-source-id: 631c9230e6242bd858c30eec56e000ce99fbf2d8
