2020-06-12 10:22:08 -07:00
|
|
|
use super::{
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
error::{Closed, ServiceError},
|
2020-06-12 11:42:28 -07:00
|
|
|
message::{self, Message},
|
|
|
|
BatchControl,
|
2020-06-12 10:22:08 -07:00
|
|
|
};
|
2020-06-12 11:42:28 -07:00
|
|
|
use futures::future::TryFutureExt;
|
2020-06-12 10:22:08 -07:00
|
|
|
use pin_project::pin_project;
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
use std::sync::{Arc, Mutex};
|
2020-06-12 11:42:28 -07:00
|
|
|
use tokio::{
|
|
|
|
stream::StreamExt,
|
|
|
|
sync::mpsc,
|
|
|
|
time::{delay_for, Delay},
|
2020-06-12 10:22:08 -07:00
|
|
|
};
|
2020-06-12 11:42:28 -07:00
|
|
|
use tower::{Service, ServiceExt};
|
|
|
|
use tracing_futures::Instrument;
|
2020-06-12 10:22:08 -07:00
|
|
|
|
|
|
|
/// Task that handles processing the buffer. This type should not be used
|
|
|
|
/// directly, instead `Buffer` requires an `Executor` that can accept this task.
|
|
|
|
///
|
|
|
|
/// The struct is `pub` in the private module and the type is *not* re-exported
|
|
|
|
/// as part of the public API. This is the "sealed" pattern to include "private"
|
|
|
|
/// types in public traits that are not meant for consumers of the library to
|
|
|
|
/// implement (only call).
|
|
|
|
#[pin_project]
|
|
|
|
#[derive(Debug)]
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
pub struct Worker<T, Request>
|
2020-06-12 10:22:08 -07:00
|
|
|
where
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
T: Service<BatchControl<Request>>,
|
|
|
|
T::Error: Into<crate::BoxError>,
|
2020-06-12 10:22:08 -07:00
|
|
|
{
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
rx: mpsc::Receiver<Message<Request, T::Future>>,
|
|
|
|
service: T,
|
|
|
|
failed: Option<ServiceError>,
|
|
|
|
handle: Handle,
|
2020-06-12 11:42:28 -07:00
|
|
|
max_items: usize,
|
|
|
|
max_latency: std::time::Duration,
|
2020-06-12 10:22:08 -07:00
|
|
|
}
|
|
|
|
|
|
|
|
/// Get the error out
|
|
|
|
#[derive(Debug)]
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
pub(crate) struct Handle {
|
|
|
|
inner: Arc<Mutex<Option<ServiceError>>>,
|
2020-06-12 10:22:08 -07:00
|
|
|
}
|
|
|
|
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
impl<T, Request> Worker<T, Request>
|
2020-06-12 10:22:08 -07:00
|
|
|
where
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
T: Service<BatchControl<Request>>,
|
|
|
|
T::Error: Into<crate::BoxError>,
|
2020-06-12 10:22:08 -07:00
|
|
|
{
|
|
|
|
pub(crate) fn new(
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
service: T,
|
|
|
|
rx: mpsc::Receiver<Message<Request, T::Future>>,
|
2020-06-12 11:42:28 -07:00
|
|
|
max_items: usize,
|
|
|
|
max_latency: std::time::Duration,
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
) -> (Handle, Worker<T, Request>) {
|
2020-06-12 10:22:08 -07:00
|
|
|
let handle = Handle {
|
|
|
|
inner: Arc::new(Mutex::new(None)),
|
|
|
|
};
|
|
|
|
|
|
|
|
let worker = Worker {
|
|
|
|
rx,
|
|
|
|
service,
|
|
|
|
handle: handle.clone(),
|
2020-06-12 11:42:28 -07:00
|
|
|
failed: None,
|
|
|
|
max_items,
|
|
|
|
max_latency,
|
2020-06-12 10:22:08 -07:00
|
|
|
};
|
|
|
|
|
|
|
|
(handle, worker)
|
|
|
|
}
|
|
|
|
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
async fn process_req(&mut self, req: Request, tx: message::Tx<T::Future>) {
|
|
|
|
if let Some(ref failed) = self.failed {
|
2020-06-12 11:42:28 -07:00
|
|
|
tracing::trace!("notifying caller about worker failure");
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
let _ = tx.send(Err(failed.clone()));
|
2020-06-12 11:42:28 -07:00
|
|
|
} else {
|
|
|
|
match self.service.ready_and().await {
|
|
|
|
Ok(svc) => {
|
|
|
|
let rsp = svc.call(req.into());
|
|
|
|
let _ = tx.send(Ok(rsp));
|
|
|
|
}
|
|
|
|
Err(e) => {
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
self.failed(e.into());
|
2020-06-12 11:42:28 -07:00
|
|
|
let _ = tx.send(Err(self
|
|
|
|
.failed
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
.as_ref()
|
|
|
|
.expect("Worker::failed did not set self.failed?")
|
|
|
|
.clone()));
|
2020-06-12 11:42:28 -07:00
|
|
|
}
|
2020-06-12 10:22:08 -07:00
|
|
|
}
|
2020-06-12 11:42:28 -07:00
|
|
|
}
|
|
|
|
}
|
2020-06-12 10:22:08 -07:00
|
|
|
|
2020-06-12 11:42:28 -07:00
|
|
|
async fn flush_service(&mut self) {
|
|
|
|
if let Err(e) = self
|
|
|
|
.service
|
|
|
|
.ready_and()
|
|
|
|
.and_then(|svc| svc.call(BatchControl::Flush))
|
|
|
|
.await
|
|
|
|
{
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
self.failed(e.into());
|
2020-06-12 10:22:08 -07:00
|
|
|
}
|
2020-06-12 11:42:28 -07:00
|
|
|
}
|
2020-06-12 10:22:08 -07:00
|
|
|
|
2020-06-12 11:42:28 -07:00
|
|
|
pub async fn run(mut self) {
|
|
|
|
use futures::future::Either::{Left, Right};
|
|
|
|
// The timer is started when the first entry of a new batch is
|
|
|
|
// submitted, so that the batch latency of all entries is at most
|
|
|
|
// self.max_latency. However, we don't keep the timer running unless
|
|
|
|
// there is a pending request to prevent wakeups on idle services.
|
|
|
|
let mut timer: Option<Delay> = None;
|
|
|
|
let mut pending_items = 0usize;
|
|
|
|
loop {
|
|
|
|
match timer {
|
|
|
|
None => match self.rx.next().await {
|
|
|
|
// The first message in a new batch.
|
|
|
|
Some(msg) => {
|
|
|
|
let span = msg.span;
|
|
|
|
self.process_req(msg.request, msg.tx)
|
|
|
|
// Apply the provided span to request processing
|
|
|
|
.instrument(span)
|
|
|
|
.await;
|
|
|
|
timer = Some(delay_for(self.max_latency));
|
|
|
|
pending_items = 1;
|
|
|
|
}
|
|
|
|
// No more messages, ever.
|
|
|
|
None => return,
|
|
|
|
},
|
|
|
|
Some(delay) => {
|
|
|
|
// Wait on either a new message or the batch timer.
|
|
|
|
match futures::future::select(self.rx.next(), delay).await {
|
|
|
|
Left((Some(msg), delay)) => {
|
|
|
|
let span = msg.span;
|
|
|
|
self.process_req(msg.request, msg.tx)
|
|
|
|
// Apply the provided span to request processing.
|
|
|
|
.instrument(span)
|
|
|
|
.await;
|
|
|
|
pending_items += 1;
|
|
|
|
// Check whether we have too many pending items.
|
|
|
|
if pending_items >= self.max_items {
|
|
|
|
// XXX(hdevalence): what span should instrument this?
|
|
|
|
self.flush_service().await;
|
|
|
|
// Now we have an empty batch.
|
|
|
|
timer = None;
|
|
|
|
pending_items = 0;
|
|
|
|
} else {
|
|
|
|
// The timer is still running, set it back!
|
|
|
|
timer = Some(delay);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
// No more messages, ever.
|
|
|
|
Left((None, _delay)) => {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
// The batch timer elapsed.
|
|
|
|
Right(((), _next)) => {
|
|
|
|
// XXX(hdevalence): what span should instrument this?
|
|
|
|
self.flush_service().await;
|
|
|
|
timer = None;
|
|
|
|
pending_items = 0;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
2020-06-12 10:22:08 -07:00
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
fn failed(&mut self, error: crate::BoxError) {
|
|
|
|
// The underlying service failed when we called `poll_ready` on it with the given `error`. We
|
|
|
|
// need to communicate this to all the `Buffer` handles. To do so, we wrap up the error in
|
|
|
|
// an `Arc`, send that `Arc<E>` to all pending requests, and store it so that subsequent
|
|
|
|
// requests will also fail with the same error.
|
2020-06-12 10:22:08 -07:00
|
|
|
|
|
|
|
// Note that we need to handle the case where some handle is concurrently trying to send us
|
|
|
|
// a request. We need to make sure that *either* the send of the request fails *or* it
|
|
|
|
// receives an error on the `oneshot` it constructed. Specifically, we want to avoid the
|
|
|
|
// case where we send errors to all outstanding requests, and *then* the caller sends its
|
|
|
|
// request. We do this by *first* exposing the error, *then* closing the channel used to
|
|
|
|
// send more requests (so the client will see the error when the send fails), and *then*
|
|
|
|
// sending the error to all outstanding requests.
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
let error = ServiceError::new(error);
|
2020-06-12 10:22:08 -07:00
|
|
|
|
|
|
|
let mut inner = self.handle.inner.lock().unwrap();
|
|
|
|
|
|
|
|
if inner.is_some() {
|
|
|
|
// Future::poll was called after we've already errored out!
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
2020-06-17 17:00:19 -07:00
|
|
|
*inner = Some(error.clone());
|
2020-06-12 10:22:08 -07:00
|
|
|
drop(inner);
|
|
|
|
|
|
|
|
self.rx.close();
|
|
|
|
|
2020-06-12 11:42:28 -07:00
|
|
|
// By closing the mpsc::Receiver, we know that that the run() loop will
|
|
|
|
// drain all pending requests. We just need to make sure that any
|
|
|
|
// requests that we receive before we've exhausted the receiver receive
|
|
|
|
// the error:
|
2020-06-12 10:22:08 -07:00
|
|
|
self.failed = Some(error);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
impl Handle {
|
|
|
|
pub(crate) fn get_error_on_closed(&self) -> crate::BoxError {
|
2020-06-12 10:22:08 -07:00
|
|
|
self.inner
|
|
|
|
.lock()
|
|
|
|
.unwrap()
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
.as_ref()
|
|
|
|
.map(|svc_err| svc_err.clone().into())
|
2020-06-12 10:22:08 -07:00
|
|
|
.unwrap_or_else(|| Closed::new().into())
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
Revert #500 (generic errors in tower-batch).
Unfortunately, since the Batch wrapper was changed to have a generic error
type, when wrapping it in another Service, nothing constrains the error type,
so we have to specify it explicitly to avoid an inference hole. This is pretty
unergonomic -- from the compiler error message it's very unintuitive that the
right fix is to change `Batch::new` to `Batch::<_, _, SomeError>::new`.
The options are:
1. roll back the changes that make the error type generic, so that the error
type is a concrete type;
2. keep the error type generic but hardcode the error in the default
constructor and add an additional code path that allows overriding the
error.
However, there's a further issue with generic errors: the error type must be
Clone. This problem comes from the fact that there can be multiple Batch
handles that have to share access to errors generated by the inner Batch
worker, so there's not a way to work around this. However, almost all error
types aren't Clone, so there are fairly few error types that we would be
swapping in.
This suggests that in case (2) we would be maintaining extra code to allow
generic errors, but with restrictive enough generic bounds to make it
impractical to use generic error types. For this reason I think that (1) is a
better option.
2020-07-15 21:42:57 -07:00
|
|
|
impl Clone for Handle {
|
|
|
|
fn clone(&self) -> Handle {
|
2020-06-12 10:22:08 -07:00
|
|
|
Handle {
|
|
|
|
inner: self.inner.clone(),
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|