feat(server): Make sleep_on_error duration configurable and enable it per default #1457
Conversation
src/server/mod.rs
Outdated
| @@ -683,14 +685,14 @@ impl Stream for AddrIncoming { | |||
| return Ok(Async::Ready(Some(AddrStream::new(socket, addr)))); | |||
| }, | |||
| Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => return Ok(Async::NotReady), | |||
| Err(ref e) if self.sleep_on_errors => { | |||
| Err(ref e) if self.sleep_on_errors.is_some() => { | |||
There was a problem hiding this comment.
I wonder if this can be rewritten as:
Err(ref e) if let Some(delay) = self.sleep_on_errors => {so you don't need to use .unwrap() later.
There was a problem hiding this comment.
Tried that, but the compiler does not allow let in a match arm, as far as I can see.
There was a problem hiding this comment.
Could be done like this, right?
match self.listener.accept() {
// Ok ...
Err(e) => {
if let Some(delay) = self.sleep_on_errors {
// continue or delay
}
return Err(e);
}
}
|
The test fail on AppVeyor does not seem related to this change. |
| /// This can be disabled by setting the duration to None. | ||
| /// | ||
| /// Default is 10ms. | ||
| pub fn sleep_on_errors(&mut self, duration: Option<Duration>) -> &mut Self { |
There was a problem hiding this comment.
We couldn't make this change for 0.11.x, but could be in 0.12.
src/server/mod.rs
Outdated
| @@ -683,14 +685,14 @@ impl Stream for AddrIncoming { | |||
| return Ok(Async::Ready(Some(AddrStream::new(socket, addr)))); | |||
| }, | |||
| Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => return Ok(Async::NotReady), | |||
| Err(ref e) if self.sleep_on_errors => { | |||
| Err(ref e) if self.sleep_on_errors.is_some() => { | |||
There was a problem hiding this comment.
Could be done like this, right?
match self.listener.accept() {
// Ok ...
Err(e) => {
if let Some(delay) = self.sleep_on_errors {
// continue or delay
}
return Err(e);
}
}
klausi
force-pushed
the
sleep_on_errors_configurable
branch
from
a815d74
to
a420ebd
Compare
|
@seanmonstar good idea with the |
klausi
force-pushed
the
sleep_on_errors_configurable
branch
from
05fee87
to
48fc125
Compare
|
So, the changes look right! Thanks! However, as mentioned in a line comment, this is a breaking change, so couldn't be done in 0.11. I'd say this could be a PR against the 0.12.x branch, but with that switching to the new tokio, I'm unsure where timeouts will come from at the moment... |
|
I think we can put this on hold until the new Tokio is integrated and then rewrite this wherever it fits in. |
|
With #1488 merged, a variant of this is now in place. Thanks! |
For issue #1455 .
This is a minor API break because I'm changing the type of the sleep_on_error() method. This method exists only for a very short amount of time and Hyper is pre-1.0 so changing this might be ok?