GH-818 Are Chunked Responses Being Handled Wrong? #682
Conversation
dnwiebe
changed the title
| use tokio::prelude::Future; | ||
|
|
||
| pub const CRASH_KEY: &str = "PROXYSERVER"; | ||
| pub const RETURN_ROUTE_TTL: Duration = Duration::from_secs(120); |
There was a problem hiding this comment.
Now there are two TtlHashMaps, so we need two TTLs.
| .get(&msg.stream_key) | ||
| .unwrap_or_else(|| { | ||
| panic!("AddRouteResultMessage Handler: stream key: {} not found within dns_failure_retries", msg.stream_key); | ||
| }); |
There was a problem hiding this comment.
Took out the panic, replaced it with an error log. This panic doesn't happen because of something that arrives from outside, but it does happen because of something that doesn't arrive from outside; that should be against The Rule as well.
| let return_route_id = match self.rri_from_remaining_route(&msg.remaining_route) { | ||
| Some(rri) => rri, | ||
| None => return, // TODO: Eventually we'll have to do something better here, but we'll probably need some heuristics. | ||
| }; |
There was a problem hiding this comment.
get_return_route_info() has been modified to accept only the return_route_id (a u32) rather than the whole Route; so here we're pulling the return_route_id out of the Route we have.
| } | ||
| } | ||
|
|
||
| fn get_return_route_info( |
There was a problem hiding this comment.
This is hard to read, but I split the original get_return_route_info() into two:
rri_from_remaining_route() that pulls the ID out of the Route
get_return_route_info() (accepting an ID, not a whole Route) to traverse the TtlHashMaps
| source: &str, | ||
| ) -> Option<Rc<AddReturnRouteMessage>> { | ||
| match self.route_ids_to_return_routes_first_chance.remove(&return_route_id) { | ||
| Some(rri) => { |
There was a problem hiding this comment.
There's no use case in which we leave a matching RRI in the _first_chance map. If we find it, we'll transfer it over to the _stragglers map, where it is subject to a much shorter TTL and will probably expire and be killed unless it's part of a stream.
node/src/proxy_server/mod.rs
Outdated
| self.route_ids_to_return_routes_stragglers.insert(return_route_id, (*rri).clone()); | ||
| Some(rri) | ||
| }, | ||
| None => match self.route_ids_to_return_routes_stragglers.get(&return_route_id) { |
There was a problem hiding this comment.
If we don't find matching RRI in the _first_chance map, we immediately look in the _stragglers map.
| self | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The following tests address get_return_route_info() directly (white-box tests) and make sure it operates as intended.
| #[test] | ||
| #[should_panic( | ||
| expected = "AddRouteResultMessage Handler: stream key: AAAAAAAAAAAAAAAAAAAAAAAAAAA not found within dns_failure_retries" | ||
| )] |
There was a problem hiding this comment.
Converted panic to error log
| let stream_key = StreamKey::make_meaningful_stream_key(test_name); | ||
| subject | ||
| .keys_and_addrs | ||
| .insert(stream_key.clone(), socket_addr.clone()); |
There was a problem hiding this comment.
The rest of this file is just renames.
No description provided.