2020 tower spring cleaning

[jonhoo]

It's about time we made a serious push to make tower "ready for public use". Here is a list of what should happen to make that a reality:

• Merge all middleware crates (everything except tower-service) into a single tower crate like tokio did. We should add features that enable and disable different middleware, with all features being off by default.
• Unify the error bounds across all middleware to all use either From or Into (see Migrate crates to use Error: From<S::Error> #387). We should also think about Add section about not including sources in Display impl rust-lang/api-guidelines#210, and whether we want to stop printing the source when we print our errors.
• Make a decision on how to move forward with poll_ready (see Disarming Service after successful poll_ready #408, Make poll_ready() return a token which call() consumes #412, and Undoing a Sink::poll_ready rust-lang/futures-rs#2109).
• Update CI to match new crate org, and include basic amenities (like cargo-deny).
• Update all docs to at least have all items documented (#![deny(missing_docs)]) and basic usage examples + module docs. We should also tidy up the README + crate docs + metadata while we're at it. See also Document the library #33.
• Update all code with doc_cfg when running on docs.rs (see docs: annotate io mod with doc_cfg tokio-rs/tokio#1808), and enable all features on docs.rs (like docs: add docs.rs metadata to build with all features tokio-rs/tokio#1471)
• Figure out how to manage modules that previously had publish = false. We could make them non-pub, though that would make the tests stop working...
• Add free function shorthands for some of the various modules (à la warp), such as tower::balance::p2c::layer() or tower::balance::p2c() as a shorthand for tower::balance::p2c::MakeBalanceLayer::new().
• Consider if Target should be renamed to a more understandable name. Perhaps Endpoint?

For the per-crate/module docs cleanup, here are the things we need to do, what we have done, and who's going to do each one. If you want to make a pass over one of these, please post here to claim so we don't duplicate the work. As you make your pass, please also create issues for implementation and API issues you spot (for example, limit should probably not vendor parts of tokio::sync).

• tower-layer
• tower-service
• tower-test (@jonhoo)
• tower itself
  • tower/balance (Spring cleaning for tower::balance #449)
  • tower/buffer (Tidy up tower::buffer #444)
  • tower/builder
  • tower/discover (Change Discover to be a sealed trait #443)
  • tower/filter
  • tower/hedge
  • tower/limit
  • tower/load (Tidy up tower::load #445)
  • tower/load_shed
  • tower/make
  • tower/ready_cache
  • tower/reconnect
  • tower/retry
  • tower/spawn_ready
  • tower/steer
  • tower/timeout
  • tower/util

Things we would also like to do, but are not blockers:

• Create more GitHub issue labels, especially one for each module.
• Triage all GitHub issues, apply appropriate labels, close stale issues.
• Triage all current GitHub PRs, and make steps to move each one forward (may just consist of pinging the author).

[hawkw]

* Update _all_ docs to at least have all items documented (`#![deny(missing_docs)]`) and basic usage examples + module docs. We should also tidy up the `README` + crate docs + metadata while we're at it. See also #33.

We should probably also add the doc_cfg nightly feature to docs.rs builds, and add the appropriate attributes to feature-flagged APIs (similar to tonic, tokio, etc).

[jonhoo]

I've updated the v0.3.x branch to point to the current head of master (0f9eb64). Going forward, master will be where we develop the "new and shiny", and then any maintenance of the 0.3 releases will happen on v0.3.x. The thinking is that this will incentivize us to get a new tower release out as soon as we can, to minimize the time that there is a mismatch between master and the code that people get through crates.io.

[jonhoo]

We now have tons of fancy new labels.

[davidbarsky]

@jonhoo A few notes on poll_ready:

• I think Tower should move to the model with an explicit semaphore, but I'm not sure this requires a new associated type.
• I'd also like to see a corresponding high-level trait (called Dispatch, maybe? Bikesheds welcome) which does not expose a public poll_ready method and changes the call method to take &self introduced alongside the new Service trait, as Service is getting more and more low-level.
• If tower::Service moves to a semaphore-based approach, is there any value in keeping &mut self around? Tokens + &self could reduce how many .clone()s need to be written.

[jonhoo]

I did triage!

[jonhoo]

@davidbarsky Can you say more about what you mean by "explicit semaphore". Are you thinking about #412, or something else? Also, I'm not sure I follow why Dispatch would be useful?

[jonhoo]

Also, I wonder if we should keep the discussion of poll_ready to #408. Or maybe we (you? :p) should make a new meta-issue specifically about poll_ready? And then arguably close #408 and #412, since they are both specific proposals, both of which can be folded into one bigger discussion around poll_ready.

[hawkw]

@davidbarsky

• I'd also like to see a corresponding high-level trait (called Dispatch, maybe? Bikesheds welcome) which does not expose a public poll_ready method and changes the call method to take &self introduced alongside the new Service trait, as Service is getting more and more low-level.

This sounds a lot like the ReadyService trait that was added in #25 and then removed in #50. IIRC, there were some issues with ReadyService that lead to its removal. We should probably do some thinking about whether those issues still apply before reintroducing a similar trait.

• I think Tower should move to the model with an explicit semaphore, but I'm not sure this requires a new associated type.

What do you mean by "explicit semaphore"? If you're proposing adding a semaphore-based concurrency control mechanism to every service, I'm not sure how I feel about that. Many services (including most middleware services) don't require their own concurrency control, just propagating backpressure from lower-level services up the stack. Am I understanding what you mean correctly?

[davidbarsky]

@jonhoo Yeah, some sort of dedicated issue for poll_ready is a good idea. I'll create one later today once I clear this morning's round of meetings.

@hawkw

This sounds a lot like the ReadyService trait that was added in #25 and then removed in #50. IIRC, there were some issues with ReadyService that lead to its removal. We should probably do some thinking about whether those issues still apply before reintroducing a similar trait.

Ah, good to know. I'll take a look at that those two issues.

What do you mean by "explicit semaphore"? If you're proposing adding a semaphore-based concurrency control mechanism to every service, I'm not sure how I feel about that. Many services (including most middleware services) don't require their own concurrency control, just propagating backpressure from lower-level services up the stack. Am I understanding what you mean correctly?

By "explicit semaphore", I was referring to the proposal outlined in #412. Backpressure would be propagated by passing a token up through the Service stack rather than relying on each Service to call poll_ready() themselves. That being said, the Lock approach you mentioned that exists in Linkerd might be a compelling alternative.

[jonhoo]

(@davidbarsky see also rust-lang/futures-rs#2109, which is my primary consideration for poll_ready)

[jonhoo]

Fixed the docs feature annotations and publish = false in #442.

[hawkw]

Now that 0.4 has been shipped, I think we can call this "done"!