Skip to content

Commit

Permalink
rename and add comments
Browse files Browse the repository at this point in the history
  • Loading branch information
conradludgate committed Jun 25, 2024
1 parent 6db02f0 commit 23f004a
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 20 deletions.
6 changes: 3 additions & 3 deletions proxy/src/proxy/connect_compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use pq_proto::StartupMessageParams;
use tokio::time;
use tracing::{error, info, warn};

use super::retry::CouldRetry2;
use super::retry::ShouldRetryWakeCompute;

const CONNECT_TIMEOUT: time::Duration = time::Duration::from_secs(2);

Expand Down Expand Up @@ -106,7 +106,7 @@ pub async fn connect_to_compute<M: ConnectMechanism, B: ComputeConnectBackend>(
connect_to_compute_retry_config: RetryConfig,
) -> Result<M::Connection, M::Error>
where
M::ConnectError: CouldRetry + CouldRetry2 + std::fmt::Debug,
M::ConnectError: CouldRetry + ShouldRetryWakeCompute + std::fmt::Debug,
M::Error: From<WakeComputeError>,
{
let mut num_retries = 0;
Expand Down Expand Up @@ -141,7 +141,7 @@ where

error!(error = ?err, "could not connect to compute node");

let node_info = if !node_info.cached() || !err.should_retry_database_address() {
let node_info = if !node_info.cached() || !err.should_retry_wake_compute() {
// If we just recieved this from cplane and dodn't get it from cache, we shouldn't retry.
// Do not need to retrieve a new node_info, just return the old one.
if should_retry(&err, num_retries, connect_to_compute_retry_config) {
Expand Down
25 changes: 15 additions & 10 deletions proxy/src/proxy/retry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ use std::{error::Error, io};
use tokio::time;

pub trait CouldRetry {
/// Returns true if the error could be retried
fn could_retry(&self) -> bool;
}

pub trait CouldRetry2 {
fn should_retry_database_address(&self) -> bool;
pub trait ShouldRetryWakeCompute {
/// Returns true if we need to invalidate the cache for this node.
/// If false, we can continue retrying with the current node cache.
fn should_retry_wake_compute(&self) -> bool;
}

pub fn should_retry(err: &impl CouldRetry, num_retries: u32, config: RetryConfig) -> bool {
Expand Down Expand Up @@ -36,8 +39,8 @@ impl CouldRetry for tokio_postgres::error::DbError {
)
}
}
impl CouldRetry2 for tokio_postgres::error::DbError {
fn should_retry_database_address(&self) -> bool {
impl ShouldRetryWakeCompute for tokio_postgres::error::DbError {
fn should_retry_wake_compute(&self) -> bool {
use tokio_postgres::error::SqlState;
// Here are errors that happens after the user successfully authenticated to the database.
// TODO: there are pgbouncer errors that should be retried, but they are not listed here.
Expand Down Expand Up @@ -65,11 +68,13 @@ impl CouldRetry for tokio_postgres::Error {
}
}
}
impl CouldRetry2 for tokio_postgres::Error {
fn should_retry_database_address(&self) -> bool {
impl ShouldRetryWakeCompute for tokio_postgres::Error {
fn should_retry_wake_compute(&self) -> bool {
if let Some(db_err) = self.source().and_then(|x| x.downcast_ref()) {
tokio_postgres::error::DbError::should_retry_database_address(db_err)
tokio_postgres::error::DbError::should_retry_wake_compute(db_err)
} else {
// likely an IO error. Possible the compute has shutdown and the
// cache is stale.
true
}
}
Expand All @@ -85,10 +90,10 @@ impl CouldRetry for compute::ConnectionError {
}
}
}
impl CouldRetry2 for compute::ConnectionError {
fn should_retry_database_address(&self) -> bool {
impl ShouldRetryWakeCompute for compute::ConnectionError {
fn should_retry_wake_compute(&self) -> bool {
match self {
compute::ConnectionError::Postgres(err) => err.should_retry_database_address(),
compute::ConnectionError::Postgres(err) => err.should_retry_wake_compute(),
// the cache entry was not checked for validity
compute::ConnectionError::TooManyConnectionAttempts(_) => false,
_ => true,
Expand Down
6 changes: 3 additions & 3 deletions proxy/src/proxy/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::error::ErrorKind;
use crate::{http, sasl, scram, BranchId, EndpointId, ProjectId};
use anyhow::{bail, Context};
use async_trait::async_trait;
use retry::{retry_after, CouldRetry2};
use retry::{retry_after, ShouldRetryWakeCompute};
use rstest::rstest;
use rustls::pki_types;
use tokio_postgres::config::SslMode;
Expand Down Expand Up @@ -443,8 +443,8 @@ impl CouldRetry for TestConnectError {
self.retryable
}
}
impl CouldRetry2 for TestConnectError {
fn should_retry_database_address(&self) -> bool {
impl ShouldRetryWakeCompute for TestConnectError {
fn should_retry_wake_compute(&self) -> bool {
true
}
}
Expand Down
8 changes: 4 additions & 4 deletions proxy/src/serverless/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
intern::EndpointIdInt,
proxy::{
connect_compute::ConnectMechanism,
retry::{CouldRetry, CouldRetry2},
retry::{CouldRetry, ShouldRetryWakeCompute},
},
rate_limiter::EndpointRateLimiter,
Host,
Expand Down Expand Up @@ -194,10 +194,10 @@ impl CouldRetry for HttpConnError {
}
}
}
impl CouldRetry2 for HttpConnError {
fn should_retry_database_address(&self) -> bool {
impl ShouldRetryWakeCompute for HttpConnError {
fn should_retry_wake_compute(&self) -> bool {
match self {
HttpConnError::ConnectionError(e) => e.should_retry_database_address(),
HttpConnError::ConnectionError(e) => e.should_retry_wake_compute(),
// we never checked cache validity
HttpConnError::TooManyConnectionAttempts(_) => false,
_ => true,
Expand Down

0 comments on commit 23f004a

Please sign in to comment.