Redesign: futures, smaller core, easier to contribute, better tested, and much more

[passcod]

The below is saved as a historical document, but many things are incorrect now. The best place to see where this is at is the next branch and other issues associated with the Next milestone. This issue will be kept open until the first vNext beta is out so that all interested get notified. For a high level presentation, see instead this draft announce.

---

This is a write up of stuff that's mostly in my head, and partially written down in code. If something seems missing, ask! Either I've overlooked it and it will need to be considered, or I've not written it down, in which case I'll need to do that. Reddit thread.

(Issues that I'm looking to fix by design with this: #112 (see the Event enums), #110, #62, #64, #66, #67, #115 (see the Events discussion), probably improve #63 (see the high-level plan), various other issues and incompatibilities we've hit in the past, velocity/bugfree-ness as backend bugfixes can be released into general usage independently of the main Notify crate).

Goals / overview / bullet points

• All backends implement a single, well-defined, small low-level API.
• Backends declare what they support, and Notify caters to them.
• No more having two APIs (raw and debounced), they're both replaced by a single high-level API that can do both (and more).
• Notify is many crates! Consumers can choose to use the general API, or they can compile against only a single backend and use its low-level API.
• Backends can be chosen and switched at runtime.

Advanced/Beyond goals/features:

These are brief descriptions of features that would be possible to implement after this redesign work is complete, and the rest of this document does not depend on these.

• Backends could (alternatively to being included with Notify itself) be attached to a Notify Daemon "component", that makes the low-level interface available over a socket or IPC channel or bus or something, and advertises their presence. Notify would be able to speak that protocol, and therefore would be able to use backends that aren't compiled in.

  This is an alternate way to use backends and would not replace the primary way (directly using the Rust interface of the backend through compilation), and there would be a Cargo feature to disable it entirely at compile time.

  The benefits of that would be three-fold:

  1. People could develop third-party backends and use them / distribute them / make them available to Notify programs without going through us, or even without making them open-source.
  2. Notify itself could cooperate with other Notify instances on the system through exposing/consuming its backends as daemons.
  3. Other programs written in other programming languages could directly use Notify backends without needing Rust bindings. Note that using Notify's high-level interface from other languages is a separate topic.

  This still needs a bunch of details worked in, like security.

• Several backends could be used concurrently. For example, FSEvent and polling and kqueue on Mac systems, or INotify and kqueue on BSD systems with the INotify bridge, or…

• Bindings of Notify's high-level interface for other languages. For example, I'd love to do a Node.js native module that exposes Notify to that ecosystem. Also, Ruby and Python. @dherman's Rust Bindings projects will help there.

Low-level

The Notify Backend interface

A backend is defined as being a crate that implements the Notify Backend interface. This is a Rust trait, but it's also much stricter than that. The crate must be a library that must expose a struct that must be named Backend. It must implement the NotifyBackend trait correctly, and it must declare its capabilities, and it must not try to implement more capabilities that its native interface does not support by itself (unless it's a polling backend, as they do everything themselves anyway). It must be named notify-backend-{name} and {name} must be lowercase-with-hyphens. If it wants to be considered for inclusion as first-tier, compiled-in-the-default-Notify-distribution, it must be on crates.io, with all the further restrictions that implies, and it must compile on Rust stable.

The actual trait is four methods:

• new(paths: Vec<PathBuf>) -> Result<Self>
• capabilities() -> Vec<Capability>
• the Drop trait
• the Stream trait

The idea is that instead of having backends be mutable structures that need to manage adding and removing paths and keeping track of what to send to whom or whatnot... a backend is immutable, constructed from a set of paths. It initialises its native or underlying engine when it's constructed, and then de-initialises when it's Dropped. In between that, it provides events on a futures::Stream, when polled. That's all.

The Notify Backend facilities

The notify-backend crate provides:

• The Backend trait
• A Result type and its own Error type, that supports a wider range of error sources than just io::Error, along with a bunch of From<t> for Error implementations to make using it easier.
• An enum of Capabilities along with fairly extensive documentation on what they mean, so go read that there.
• The Event type, which contains a nested-enums-based machine-readable description of exactly what kind of event it is. This is actually pretty clever, if I may say so myself, as it allows passing down a lot of information while also supporting a single common set of top-level events (create, remove, modify, and optionally access) that can be supported by every backend, avoiding situations like wanting to remove CLOSE_WRITE because it's a Linux-only thing, but not wanting to drop potentially useful functionality either. When writing the hierarchy, I went through all filesystem eventing native APIs I could find, even historical dead ones that date back to IRIX, and built the system so that it could support all events they provide.
• Ready-made Error and Item types for the Stream trait, to cut down on boilerplate.
• A Buffer structure that can, well, buffer events and exposes a Stream poll(). The purpose is two-fold: One, some native APIs don't support returning a single event and keeping the rest, they just give you everything they've got… but Stream only supports a single Item per poll() call, so the remaining has to be buffered somewhere, and Buffer is there to provide that facility in a common manner to avoid boilerplate. Two, native APIs that do have a queue generally behave badly/annoyingly when the queue overflows. Instead of letting that happen, we attempt to take many events from the queue by default, and stick them in our Buffer. Our Buffer silently drops excess events at some threshold (by default 16384 events) and will not crash or Drop the backend or something silly like that, avoiding overflows on the native side.
• A prelude to import all that a backend will need to be implemented, again avoiding lots of boilerplate.
• And finally, a standard suite of tests that run against a backend.

As you've noticed, a lot of that is designed to reduce boilerplate. If you have other ideas on how boilerplate can be further reduced, they'd be appreciated… except a macro. I think macros automatically make things harder to debug, and I already use one for the tests. We want to make things easier on contributors/backend writers.

As it is, have a look at the kqueue backend to get an idea of how simple it is now to contribute support for a new native API. I wrote this one in three days using a virtual machine, two man pages, and having zero prior experience with BSD.

Progress

See the readme in the next branch.

High-level

(I have this figured out at a high level, but I don't have implementations or drafts yet.)

This part is made of a series of layers building up to a higher-level interface.

Normalising backends

The first layer looks at the backends' declared capabilities, and adds the missing ones on top if need be. For example, if a backend cannot watch folders recursively, this layer will make sure to recurse through the folder trees itself. Or if a backend can only watch folders and files, but not "all the files within a folder" (like kqueue), this layer will recurse through the trees and also pass through the files.

Only a subset of all Capabilities will be added on like that. For instance, the WatchEntireFilesystem capability will not be implemented, as that would be much too intensive and receive too little usage anyway.

Managing backends

This layer holds a set of pathspecs and a set of available backends. It assigns pathspecs to backends (how? following what precedence, logic?), and when they change (most often the pathspecs, but might also be the backends, for example for dynamic / non-compiled-in backends) it Drops and creates backend instances as needed.

Ideas for assignation logic:

• Hardcoded preference (polling at bottom, native at top, and for e.g. macOS FSEvents before kqueue)
• If backends return an Err() on ::new(), fallback to another backend
• ~If a backend fails to send any events in {some time} after creation, try another?~ Probably a bad idea.
• If a path is detected (how? resolving mounts?) as being on a network filesystem, use a backend more amenable to long latencies

Resolving common paths

This layer takes the pathspecs (combination of a Path and a RecursiveMode) given by the library consumer, resolves them (including following symlinks), and figures out if any overlap others. Then it produces a list of pathspecs that is as optimal as it can:

• if two or more specs have the same path, including in the case of symlinks, only return the one with the widest recursive mode
• if a spec is contained in another recursive one, discard the former
• if two recursive specs overlap (through a symlink) somewhere down the tree… (TODO: figure something out)
• etc

Filtering events based on pathspecs

Given the previous layer might be getting the backends to watch wider paths than what pathspecs we'd been given, this layer discards any events that don't match the original pathspecs.

Handling events

This is the core of the top interface. Basically, the idea for the top API is that the consumer/developer will give us one or more pathspecs, and a single handler callback function. The function is given an Event and a Memory (a mutable struct or something to hold whatever the callback might want to remember between calls, safely), and returns an Option<Event>. If that's None, the event is discarded and not emitted further, if it's Some(e), then e is emitted. (To allow for new synthetic events to be created by the callback.)

While technically that would be sufficient to make use of Notify, we'll also provide a bunch of builder/utility functions to help build that callback, or provide common patterns. For example:

• Composers that take two or more callbacks and compose them in some way (f() && g(), f() || g(), f(g())…)
• Debouncing on either (leading or trailing) edge
• The machinery to figure out renames
• Path or filename filtering based on regexp or globs
  • Special mention for resolving gitignore-like globlists
• etc

Optional: compatibility layer

Given the above interface, it should be possible to simulate the v4 API. This could allow us to publish a v4 that uses this redesigned version under the scenes without breaking its API. Similarly for v3 or even v2 (depending on usage). That way, we'd only have to maintain the one, modern, codebase.

[hannobraun]

I've read through this and can't see anything terribly wrong. Then again, I have never though a lot about this kind of library beyond the basic "I need to monitor some files" use case. As such, I'm not sure if the flexibility you mention (switching backends at runtime) is overkill or has valid use cases.

I'm mainly commenting because I just started an effort to polish and modernize inotify-rs. The main points I'm considering:

• Better story around sync/async. Not sure about that yet, but I'm looking into futures.
• Splitting the FFI module into a separate crate
• Cleanup, documentation, etc.

I very much plan to keep it as a low-level library that just exposes the capability of inotify.

I was planning on porting notify and requesting your feedback before I release anything anyway, but in light of this issue, I though I'd give you the heads-up now.

[passcod]

Thanks for the read-through 💛

As such, I'm not sure if the flexibility you mention (switching backends at runtime) is overkill or has valid use cases.

Yeah, it does seem like something that's a wild idea, but I've recently had to implement basically that manually on top of notify… that's where that came from.

Given I'm using futures in the backend interface here, having the inotify crate expose a futures::Stream (I suppose?) API would certainly be helpful in further reducing complexity (I'd just have to do the event translation)… although I wonder how it would work with the read() call returning multiple events. (See the point on the Buffer utility provided to backends.)

As a consumer of your crate, what'll happen is that I'll keep Notify v4 on the current version, and Notify v.Next (this redesign) will use your new / improved library. When you release or are ready, let me know :) if I'm working around that code at the time, I'll probably just adapt my side of it on the spot.

[hannobraun]

having the inotify crate expose a futures::Stream (I suppose?)

I suppose so too, but I'm not sure yet. Originally I thought I just need to return an Iterator instead of a slice, but when I started looking into it and saw that that blocking/nonblocking trickery... well, it's obvious that the library was writtern by a Rust beginner in 2014. Time to update :-)

although I wonder how it would work with the read() call returning multiple events. (See the point on the Buffer utility provided to backends.)

I don't know yet. I guess it makes sense to keep it simple on my side, if notify can translate that stuff anyway. Then again, I don't want to make it too unfriendly for any direct consumers.

We'll see. I'll think of something once I had a chance to look into it some more.

When you release or are ready, let me know :) if I'm working around that code at the time, I'll probably just adapt my side of it on the spot.

I will!

[jugglerchris]

I'm looking forward to this; I have an application using futures-rs which needs filesystem notifications, so I might start trying it out when there's something usable. If an early version isn't far off then I may as well wait for that rather than wrap threads around the current API.

[nagisa]

My comments:

• Looking at the implementation of kqueue notifier, the notifications are still not really-asynchronous. Instead, it seems, like system is polled, quite literally, ever so often, despite all of major APIs (except maybe FSEvents?) being possible to integrate with the main epoll/select/kqueue (e.g. tokio) loop.
  • side note: I was writing a library that did integrate before I realised that all of these APIs are plain broken in how they expose the events and there’s no use in even bothering to use them;

• If a backend fails to send any events in {some time} after creation, try another?

Why? How are you even going to detect that the backend does not work instead of events not occuring legitimately?

*If a path is detected (how?) as being on a network filesystem, use a backend more amenable to long latencies

You probably want at least a capability for indicating ability to watch remote and/or concurrent access file systems (this is another reason why I was writing my own library, as this didn’t do it) and automatically use a backend that is able to do that when the library is asked to monitor a remote file(system)/directory. Detection is done by resolving the real path and looking at mountpoints.

Either way, keep in mind that the underlying API behaviour is senseless in variety of nasty ways and there’s only so much you can do to paper it over.

[passcod]

If a backend fails to send any events in {some time} after creation, try another?

Why?

Because this is something that has occurred in my testing and others' and in integrating other solutions that don't have that built in. There's a few cases of notify (or other libraries) initialising without issue on a FUSE mount, but then never reporting events because the FUSE driver doesn't support them. Falling back to a polling watcher at least provides some notify support.

How are you even going to detect that the backend does not work instead of events not occuring legitimately?

That is the question, yes. I don't necessarily have all the answers at this point.

[nagisa]

There's a few cases of notify (or other libraries) initialising without issue on a FUSE mount, but then never reporting events because the FUSE driver doesn't support them. Falling back to a polling watcher at least provides some notify support.

This is the same problem as with the network filesystems. Fixing those and FUSE is essentially the same effort. I’m pretty sure that falling back to polling using some heuristics is not the right way to go about this.

[passcod]

@jugglerchris Given my current workload outside of this project I'm anticipating a testable / prerelease within two months.

[passcod]

@nagisa I've edited the section and stroke out that heuristic, thanks for your comments.

[hannobraun]

@passcod I finished my overhaul of the inotify-rs API. There's still stuff to be done, but I decided to declare victory for now, and circle back for the rest of the work later.

I just went ahead and released a new version before asking for your feedback. I figured, this way it's easier to access the documentation, and I really have no qualms about releasing more 0.x versions, should that be necessary.

One notable omission is that I'm not using futures-rs yet. I started the implementation, but ultimately decided that I needed to learn a bit more about it first. Since I have some ongoing projects that will lead me into that direction sooner or later, I figured it made sense to defer it for now.

Let me know if you have any comments or want anything changed. I'm happy to help.

[passcod]

@hannobraun I had a look at your overhaul; it looks good!

@ all, I've not had much time to work/think on this. With async in Rust stabilising, and async/await being discussed for inclusion by 2018, along with my own schedules, I think I'll pick this up again if no-one else does possibly over Yuletide.

[dfaust]

@passcod That sounds good. I have been looking at futures/tokio every now and then over the past months myself, but I'm still betting on the ergonomics (and the documentation) getting better.

[hannobraun]

@passcod Great, thanks!

[passcod]

I've started on this and will carry on throughout the next two weeks or so. I can't directly develop on macOS until end of January (by which time I'll be back at dayjob), so that might take a while, but I don't want to ship v5 until all three main platforms + polling are implemented and thoroughly tested. There will be a number of previews before then, though.

I've also taken Nagisa's comments on integrating the kernel async events into the tokio loop and figured out a way to make it work, I think. The Inotify backend uses the latest 0.5.0 from Hanno.

[cmyr]

@passcod If there's any way I can help out on the macOS side, let me know!

[mathstuf]

It'd be nice to have extra constructors for backends to trim down the events that are being looked for. For example, I use inotify with just a few bits that should generate events. I assume that would be supported as well?

[passcod]

Ideally, yes, although that would be advanced usage. But it almost certainly won't be supported in the first few releases. User-defined backends would probably be supported before that, though, so that could be a (verbose) workaround when that happens.

[mathstuf]

OK. Post-event filtering can work too, but I'd rather have the kernel do the heavy lifting :) .

[SirVer]

Came here today while trying to resolve permission denied issues in my repo after doing a recursive watch on the directory. Turns out some of my .gitignored files are making the watcher crash. I would much appreciate the 5.0 API outlined above that would allow me to filter on pathspecs and conditionally exclude stuff from the watchers list.

[passcod]

An update: I'm waiting for Tokio 0.2 to come out before continuing with this.

In the meantime I've been looking much more in depth into Tokio, both as a consumer and producer, in toy projects and experiments. Tokio Reform has brought some big changes, and I've also found that a number of details in the current v5 design don't actually make sense or need to be implemented differently with Tokio.

I've reserved some time mid-April to work on this, hoping that Tokio 0.2 will be in good way before then.

[mathstuf]

On March 10, 2018 1:01:09 AM UTC, "Félix Saparelli" [email protected] wrote:

In the meantime I've been looking much more in depth into Tokio, both as a consumer and producer, in toy projects and experiments. Tokio Reform has brought some big changes, and I've also found that a number of details in the current v5 design don't actually make sense or need to be implemented differently with Tokio.

You might be interested in my change to inotify-rs to use tokio at inotify-rs/inotify#101.

[passcod]

I have some more time and I'm starting work on this again. However, the futures and tokio ecosystem is very much in flux right now, so I think a good plan would be:

1. Keep working quietly on this with the current tokio release (aka Tokio Reform).
2. In a few months, hopefully, I should have Notify Core done and functional.
3. At the same time, Futures 0.3 should be getting released.
4. I'll keep on Tokio Reform and work on the backends.
5. When tokio goes and supports futures 0.3, I'll transition everything.
6. That will make the first v5 alpha.
7. As the ecosystem stabilises, keep testing and stabilising. Perhaps more alphas.
8. When/if we see that tokio+futures have stabilised (i.e. that it's not going to get another breaking change soon), put a beta out with an interface freeze. Everything that affects the interface from then on will have to be delayed (or branched off) to a point or major release unless severe.
9. Around that time I'll work on the compatibility wrapper for v4. At least to see if they're feasible. The idea for this is to release a v4 minor that actually use v5 under the covers but provides the correct interface.
10. Take v5 out of beta (v5.0.0) before the end of the year.

It's been a much, much longer time coming than I thought, and it's going to take a bit longer still. It will be worth it, though.

[passcod]

@mathstuf interesting! As a note, your change uses "old" tokio while Notify v5 currently uses "Reform" tokio, and these two are not compatible afaik. But this should be minor to change.

[mathstuf]

FYI, my changes were superseded by inotify-rs/inotify#105.