search

metalbear-co / mirrord

Connect your local process and your cloud environment, and run local code in cloud conditions.
https://mirrord.dev
MIT License
3.83k stars 105 forks source link

Improve filter stealer retry code. #2854

Open meowjesty opened 1 month ago

meowjesty commented 1 month ago

https://github.com/metalbear-co/mirrord/pull/2802 was merged, but there are a couple of suggestions from @Razz4780 that did not:

1. Move the original request to the error variant

Instead of having the original request in:

https://github.com/metalbear-co/mirrord/blob/258e56cfafe9aa3f9b624bd3248030e36e7f6eab/mirrord/protocol/src/tcp.rs#L327-L334

We could move it to InterceptorError, similar to how the ConnectionClosedTooSoon works:

https://github.com/metalbear-co/mirrord/blob/258e56cfafe9aa3f9b624bd3248030e36e7f6eab/mirrord/intproxy/src/proxies/incoming/interceptor.rs#L64-L92

Might reduce the amount of cloning. I've tried doing this, but it result in some annoying code https://github.com/metalbear-co/mirrord/pull/2802#issuecomment-2412202130

2. Read the first frames in Interceptor instead of in IncomingProxy

By moving

https://github.com/metalbear-co/mirrord/blob/258e56cfafe9aa3f9b624bd3248030e36e7f6eab/mirrord/intproxy/src/proxies/incoming.rs#L588-L593

To

https://github.com/metalbear-co/mirrord/blob/258e56cfafe9aa3f9b624bd3248030e36e7f6eab/mirrord/intproxy/src/proxies/incoming/interceptor.rs#L384-L393

We could handle the retry (on reset) for both request and response in the same place. Instead of having this other retry in

https://github.com/metalbear-co/mirrord/blob/258e56cfafe9aa3f9b624bd3248030e36e7f6eab/mirrord/intproxy/src/proxies/incoming.rs#L621-L645

With this change we could drop the retries field from HttpRequestFallback::Streamed. I tried to make this work, but did not manage to get the trailers right, https://github.com/metalbear-co/mirrord/pull/2802#issuecomment-2412221846

Original comments:

  1. I think you can move the original request from HttpResponseFallback::Streamed to InterceptorError::Reset. InterceptorError::ConnectionClosedTooSoon works like this and I think it results with less cloning (?)
  2. You can probably handle reading first frames from the IncomingProxy to Interceptor (and delay sending the response to IncomingProxy until you have them). You'd have retry logic in one place and you could drop retries field from HttpRequestFallback::Streamed.
linear[bot] commented 1 month ago

MBE-480 Improve filter stealer retry code.