Skip to content

Commit

Permalink
Don't reuse session when downstream errors during cache miss
Browse files Browse the repository at this point in the history
When a downstream error occurs while a cache miss is being served, the
downstream error is ignored so the upstream can continue writing to the
cache. However, it is not safe to try reusing the server session in this
case.
  • Loading branch information
drcaramelsyrup authored and eaufavor committed Oct 11, 2024
1 parent 792d5fd commit 894ca2d
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .bleep
Original file line number Diff line number Diff line change
@@ -1 +1 @@
dc97a9520b124eb464f348b0381991d8669c8d8a
8e05a8f5b9d09885e6374011c422678043a2bda0
24 changes: 14 additions & 10 deletions pingora-proxy/src/proxy_h1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl<SV> HttpProxy<SV> {
);

match ret {
Ok((_first, _second)) => (true, true, None),
Ok((downstream_can_reuse, _upstream)) => (downstream_can_reuse, true, None),
Err(e) => (false, false, Some(e)),
}
}
Expand Down Expand Up @@ -204,13 +204,14 @@ impl<SV> HttpProxy<SV> {
}

// todo use this function to replace bidirection_1to2()
// returns whether this server (downstream) session can be reused
async fn proxy_handle_downstream(
&self,
session: &mut Session,
tx: mpsc::Sender<HttpTask>,
mut rx: mpsc::Receiver<HttpTask>,
ctx: &mut SV::CTX,
) -> Result<()>
) -> Result<bool>
where
SV: ProxyHttp + Send + Sync,
SV::CTX: Send + Sync,
Expand Down Expand Up @@ -416,16 +417,19 @@ impl<SV> HttpProxy<SV> {
}
}

match session.as_mut().finish_body().await {
Ok(_) => {
debug!("finished sending body to downstream");
}
Err(e) => {
error!("Error finish sending body to downstream: {}", e);
// TODO: don't do downstream keepalive
let mut reuse_downstream = !downstream_state.is_errored();
if reuse_downstream {
match session.as_mut().finish_body().await {
Ok(_) => {
debug!("finished sending body to downstream");
}
Err(e) => {
error!("Error finish sending body to downstream: {}", e);
reuse_downstream = false;
}
}
}
Ok(())
Ok(reuse_downstream)
}

async fn h1_response_filter(
Expand Down
24 changes: 14 additions & 10 deletions pingora-proxy/src/proxy_h2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ impl<SV> HttpProxy<SV> {
);

match ret {
Ok((_first, _second)) => (true, None),
Ok((downstream_can_reuse, _upstream)) => (downstream_can_reuse, None),
Err(e) => (false, Some(e)),
}
}
Expand Down Expand Up @@ -212,13 +212,14 @@ impl<SV> HttpProxy<SV> {
(server_session_reuse, error)
}

// returns whether server (downstream) session can be reused
async fn bidirection_1to2(
&self,
session: &mut Session,
client_body: &mut h2::SendStream<bytes::Bytes>,
mut rx: mpsc::Receiver<HttpTask>,
ctx: &mut SV::CTX,
) -> Result<()>
) -> Result<bool>
where
SV: ProxyHttp + Send + Sync,
SV::CTX: Send + Sync,
Expand Down Expand Up @@ -369,16 +370,19 @@ impl<SV> HttpProxy<SV> {
}
}

match session.as_mut().finish_body().await {
Ok(_) => {
debug!("finished sending body to downstream");
}
Err(e) => {
error!("Error finish sending body to downstream: {}", e);
// TODO: don't do downstream keepalive
let mut reuse_downstream = !downstream_state.is_errored();
if reuse_downstream {
match session.as_mut().finish_body().await {
Ok(_) => {
debug!("finished sending body to downstream");
}
Err(e) => {
error!("Error finish sending body to downstream: {}", e);
reuse_downstream = false;
}
}
}
Ok(())
Ok(reuse_downstream)
}

async fn h2_response_filter(
Expand Down

0 comments on commit 894ca2d

Please sign in to comment.